CDRIVER-3755 Provide an index creation helper #1303

kevinAlbs · 2023-06-09T13:27:26Z

This PR proposes a new API for creating indexes: mongoc_collection_create_indexes_with_opts.

API for index creation

The Index Management specification includes API for creating, dropping, and listing indexes.
libmongoc includes some API for Index Management:

MONGOC_EXPORT (bool)
mongoc_collection_drop_index (mongoc_collection_t *collection,
                              const char *index_name,
                              bson_error_t *error);
MONGOC_EXPORT (bool)
mongoc_collection_drop_index_with_opts (mongoc_collection_t *collection,
                                        const char *index_name,
                                        const bson_t *opts,
                                        bson_error_t *error);
MONGOC_EXPORT (bool)
mongoc_collection_create_index (mongoc_collection_t *collection,
                                const bson_t *keys,
                                const mongoc_index_opt_t *opt,
                                bson_error_t *error) BSON_GNUC_DEPRECATED;
MONGOC_EXPORT (bool)
mongoc_collection_create_index_with_opts (mongoc_collection_t *collection,
                                          const bson_t *keys,
                                          const mongoc_index_opt_t *opt,
                                          const bson_t *opts,
                                          bson_t *reply,
                                          bson_error_t *error)
   BSON_GNUC_DEPRECATED;
MONGOC_EXPORT (bool)
mongoc_collection_ensure_index (mongoc_collection_t *collection,
                                const bson_t *keys,
                                const mongoc_index_opt_t *opt,
                                bson_error_t *error) BSON_GNUC_DEPRECATED;
MONGOC_EXPORT (mongoc_cursor_t *)
mongoc_collection_find_indexes (mongoc_collection_t *collection,
                                bson_error_t *error)
   BSON_GNUC_WARN_UNUSED_RESULT
   BSON_GNUC_DEPRECATED_FOR (mongoc_collection_find_indexes_with_opts);
MONGOC_EXPORT (mongoc_cursor_t *)
mongoc_collection_find_indexes_with_opts (mongoc_collection_t *collection,
                                          const bson_t *opts)
   BSON_GNUC_WARN_UNUSED_RESULT;

There are no non-deprecated functions to create indexes. *_create_index and *_create_index_with_opts are deprecated as of CDRIVER-2096.

Users are suggested to run the createIndexes command with the mongoc_database_write_command_with_opts in Creating Indexes guide.

CDRIVER-3755 notes wrapping drivers are required to do additional validation when the commitQuorum option is passed.

Since libmongoc does not provide an index creation helper, and generic command helpers should not be validating command specific options, this forces the user / wrapping driver to do this validation. In particular, this required additional server selection and wire version checking in the C++ driver's support of commitQuorum (CXX-1998).

This PR proposes adding a mongoc_collection_create_indexes_with_opts to loosely model the createIndexes(models: Iterable<IndexModel>, options: Optional<CreateIndexesOptions>): Iterable<String>; API from the specification.

Accepting `bson_t` for options.

The specification defines the types IndexOptions and CreateIndexOptions. These options are represented by a bson_t.

An advantage of representing the options with bson_t is forward compatibility. If new options are accepted by future server versions, no driver changes are required. And users do not need to upgrade the driver to use the new options.

Accepting bson_t for command options is consistent with other libmongoc APIs. Example: mongoc_collection_find_with_opts

Includes the caller's file and line number on error.

- return NULL if `hex` is NULL - set `len` to 0 on error This fixes a possible crash if LOCAL_MASTERKEY is not set in the example.

…th_opts`

…n_with_encryptedFields`

…_index`

`strlen` was called was before `to_encrypt.value.v_utf8.str` was assigned. This resulted in a `strlen` call with `NULL` argument.

vector-of-bool

LGTM

src/libmongoc/examples/example-create-indexes.c

Co-authored-by: vector-of-bool <[email protected]>

eramongodb

Consider also updating the following tests to use the new index helpers:

test_drop_index in test-mongoc-client-collection.c
_test_unique_index_on_keyaltnames_setup in test-mongoc-client-side-encryption.c
test_sample_indexes in test-mongoc-sample-commands.c

eramongodb · 2023-06-13T15:34:30Z

src/libmongoc/examples/example-create-indexes.c

-#include <stdlib.h>
+#include <stdlib.h> // abort
+
+#define FAIL(...)                                        \


Suggested change

#define FAIL(...) \

#define HANDLE_ERROR(...) \

For purposes of example code in documentation, suggest renaming the macro to be a bit more descriptive given the embedded code has no further context.

eramongodb · 2023-06-13T15:35:32Z

src/libmongoc/examples/example-create-indexes.c

+      if (!mongoc_collection_drop_index_with_opts (
+             coll, index_name, NULL /* opts */, &error)) {
+         bson_free (index_name);
+         bson_destroy (keys);
+         FAIL ("Failed to drop index: %s", error.message);
+      }
+      printf ("Dropped index\n");


For clarity of embedded code in documentation, suggest explicitly documenting the "success" and "failure" paths by structure:

if (mongoc_collection_create_indexes_with_opts ( coll, &im, 1, NULL /* opts */, NULL /* reply */, &error)) { printf ("Successfully created index\n"); } else { HANDLE_ERROR ("Failed to create index: %s", error.message); }

Also applies to other branches on success/failure in this example code (explicitly denote both success and failure branches, i.e. with a printf()).

I like that idea. Will apply.

eramongodb · 2023-06-13T15:39:23Z

src/libmongoc/doc/create-indexes.rst

@@ -3,23 +3,28 @@
 Creating Indexes


Should this file/page be renamed to something more general such as "Collection Indexes" or "Using Collection Indexes"? It now describes more than just their creation.

I like that idea.
Updated name from create-indexes.rst to manage-collection-indexes.rst.
Update name of example from example-create-indexes.c to example-manage-collection-indexes.c.

eramongodb · 2023-06-13T15:42:52Z

src/libmongoc/doc/create-indexes.rst


-Example
-------
+To drop an index, use :symbol:`mongoc_collection_drop_index_with_opts`. The index name may be obtained from the ``keys`` document with :symbol:`mongoc_collection_keys_to_index_string`:


Update mongoc_collection_drop_index_with_opts page with "See Also" referencing this page and/or the mongoc_collection_keys_to_index_string page.

Also suggest updating documentation page for mongoc_collection_keys_to_index_string to describe (or point to a reference describing) details concerning the keys document and its expected structure. Currently the mongoc_collection_keys_to_index_string just points back to this page without further elaboration (what's keys supposed to look like?).

eramongodb · 2023-06-13T15:44:41Z

src/libmongoc/doc/create-indexes.rst

+   :end-before: // Create an index ... end
+   :dedent: 6
+
+To list indexes, use :symbol:`mongoc_collection_find_indexes_with_opts`:


Update mongoc_collection_find_indexes_with_opts page with "See Also" referencing this page.

src/libmongoc/doc/mongoc_collection_create_indexes_with_opts.rst

eramongodb · 2023-06-13T16:00:37Z

src/libmongoc/src/mongoc/mongoc-collection.h

+
+MONGOC_EXPORT (bool)
+mongoc_collection_create_indexes_with_opts (mongoc_collection_t *collection,
+                                            mongoc_index_model_t **models,


Suggested change

mongoc_index_model_t **models,

mongoc_index_model_t *const *models,

Pointers may be const, e.g.:

mongoc_index_model_t *const im = mongoc_index_model_new (keys, NULL); if (mongoc_collection_create_indexes_with_opts ( coll, &im, 1, NULL /* opts */, NULL /* reply */, &error)) { // ... }

eramongodb · 2023-06-13T16:06:25Z

src/libmongoc/examples/client-side-encryption-helpers.h

+/* hex_to_bin parses a hexadecimal string to an array of bytes. `NULL` is
+ * returned on error. `len` is set to the number of bytes written. */


If NULL on error, suggest replacing the BSON_ASSERT in the definition with a return NULL; for consistency.

src/libmongoc/src/mongoc/mongoc-collection.c

src/libmongoc/tests/test-mongoc-collection.c

Co-authored-by: Ezra Chung <[email protected]>

…es.c`

kevinAlbs added 11 commits June 8, 2023 09:55

format mongoc-collection.c

adc4ee0

add mongoc_collection_create_indexes_with_opts

f60b9a7

make assert_match_bson a macro

40895b1

Includes the caller's file and line number on error.

fixup hex_to_bin helper

b135364

- return NULL if `hex` is NULL - set `len` to 0 on error This fixes a possible crash if LOCAL_MASTERKEY is not set in the example.

update legacy test runner to use `mongoc_collection_create_indexes_wi…

a353aff

…th_opts`

use mongoc_collection_create_indexes_with_opts in `create_collectio…

9f56e41

…n_with_encryptedFields`

use mongoc_collection_create_indexes_with_opts in `operation_create…

7dde06e

…_index`

fix placement of strlen

2b0e832

`strlen` was called was before `to_encrypt.value.v_utf8.str` was assigned. This resulted in a `strlen` call with `NULL` argument.

use mongoc_collection_create_indexes_with_opts in examples

2c12872

add documentation page for mongoc_collection_create_indexes_with_opts

4adccf9

update "Creating Indexes" guide

cc16bf6

kevinAlbs changed the title ~~CDRIVER-3755~~ CDRIVER-3755 Provide an index creation helper Jun 9, 2023

kevinAlbs marked this pull request as ready for review June 12, 2023 18:50

kevinAlbs requested review from eramongodb, vector-of-bool and adriandole June 12, 2023 18:50

vector-of-bool approved these changes Jun 12, 2023

View reviewed changes

src/libmongoc/examples/example-create-indexes.c Outdated Show resolved Hide resolved

add file name to example error message

8776adf

Co-authored-by: vector-of-bool <[email protected]>

eramongodb requested changes Jun 13, 2023

View reviewed changes

kevinAlbs and others added 11 commits June 13, 2023 16:16

fix documented type of models to include *

56af5fb

Co-authored-by: Ezra Chung <[email protected]>

simplify error handling

ad341ac

Co-authored-by: Ezra Chung <[email protected]>

revise assert message

04b8b6b

Co-authored-by: Ezra Chung <[email protected]>

use mongoc_collection_create_indexes_with_opts in more tests

989af2c

rename FAIL to HANDLE_ERROR

9418afd

document "success" and "failure" paths in example

e745334

rename guide page to "Manage Collection Indexes"

fbee4aa

rename guide file to manage-collection-indexes.rst

7d42707

rename example-create-indexes.c to `example-manage-collection-index…

4b7658f

…es.c`

add "See Also" sections

5d80add

link to createIndexes documentation for form of keys

43fcf88

kevinAlbs added 3 commits June 13, 2023 16:26

document mongoc_index_model_t

7ccd4be

change BSON_ASSERT to return NULL;

e561997

make pointer argument const

c447031

kevinAlbs requested a review from eramongodb June 14, 2023 12:51

eramongodb approved these changes Jun 14, 2023

View reviewed changes

adriandole approved these changes Jun 14, 2023

View reviewed changes

kevinAlbs merged commit ca45383 into mongodb:master Jun 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

CDRIVER-3755 Provide an index creation helper #1303

CDRIVER-3755 Provide an index creation helper #1303

Uh oh!

kevinAlbs commented Jun 9, 2023 •

edited

Loading

Uh oh!

vector-of-bool left a comment

Uh oh!

Uh oh!

eramongodb left a comment

Uh oh!

eramongodb Jun 13, 2023

Uh oh!

eramongodb Jun 13, 2023

Uh oh!

kevinAlbs Jun 13, 2023

Uh oh!

eramongodb Jun 13, 2023

Uh oh!

kevinAlbs Jun 13, 2023

Uh oh!

eramongodb Jun 13, 2023

Uh oh!

eramongodb Jun 13, 2023

Uh oh!

Uh oh!

eramongodb Jun 13, 2023

Uh oh!

eramongodb Jun 13, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

	mongoc_index_model_t **models,
	mongoc_index_model_t const models,

		/* hex_to_bin parses a hexadecimal string to an array of bytes. `NULL` is
		* returned on error. `len` is set to the number of bytes written. */

CDRIVER-3755 Provide an index creation helper #1303

CDRIVER-3755 Provide an index creation helper #1303

Uh oh!

Conversation

kevinAlbs commented Jun 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

API for index creation

Accepting bson_t for options.

Uh oh!

vector-of-bool left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eramongodb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevinAlbs commented Jun 9, 2023 •

edited

Loading

Accepting `bson_t` for options.