feat: add nulls option to createIndex#1489
Conversation
Adds an optional `nulls` option to the `createIndex` function with two possible values: `'distinct'` and `'not distinct'`. If provided, adds a `NULLS DISTINCT` or `NULLS NOT DISTINCT` clause to the index creation command.
Shinigami92
left a comment
Shinigami92
left a comment
There was a problem hiding this comment.
Please add a test/migrations/095_index_nulls.js with applicable content
|
I added the migration test file. It looks like there's an integration test failure in CI (looks like an outdated snapshot maybe?). I haven't yet been able to figure out how to run the integration tests locally - I may be missing some dependencies since I'm getting the following error: If it's just a quick snapshot update it may be quicker for someone with a working development environment to do it rather than to try to help unblock me. |
Hmm, maybe testcontainers/testcontainers-node#913 or testcontainers/testcontainers-node#793 ? Are you using EDIT: Yeah, it is the snapshot, do you want me to push the updated snapshot? @jonahkagan 
|
|
@brenoepics thanks for offering! Looking at the snapshot diff you posted, I actually realized that the test migration was missing the If you wouldn't mind posting the result of the snapshot after that change, I can validate it before you push it. Thanks for offering to help! |
|
It seems like maybe Postgresql filters out the default value "DISTINCT", so it's not showing up in the snapshot output.
|
Hm, I still don't see the "NULLS" clause in the snapshot. I wonder if Postgres filters it out if you pass in the default value ("DISTINCT"). I'm gonna try changing it to "NOT DISTINCT": 5cd4572 |
### MIGRATION 095_index_nulls (UP) ###
CREATE TABLE "t095" (
"id" integer NOT NULL
);
CREATE UNIQUE INDEX "t095_id_unique_index" ON "t095" ("id");
INSERT INTO "public"."pgmigrations" (name, run_on) VALUES ('095_index_nulls', NOW()); |
|
Ok, hmm, still not there. Last idea that I have is to remove the |
|
Actually I think I missed a build on the second run, it seems to be good now: ### MIGRATION 095_index_nulls (UP) ###
CREATE TABLE "t095" (
"id" integer
);
CREATE UNIQUE INDEX "t095_id_unique_index" ON "t095" ("id") NULLS NOT DISTINCT;
INSERT INTO "public"."pgmigrations" (name, run_on) VALUES ('095_index_nulls', NOW()); |
|
Awesome! That looks great! |
|
Last thing, Error executing:
CREATE UNIQUE INDEX "t095_id_unique_index" ON "t095" ("id") NULLS NOT DISTINCT;
^^^^
syntax error at or near "NULLS"maybe we can do something like we did at https://github.com/salsita/node-pg-migrate/blob/14cd48a6bf2a9aaac78b4d24a5142e11ee583bf3/test/migrations/093_alter_column_expression.js All other versions are working fine now. |
|
Updated with a version guard. Thanks for the pointer to the example |
Shinigami92
left a comment
Shinigami92
left a comment
There was a problem hiding this comment.
🚀 nicely done
I'm glad that I requested to add a migration test 😅
@brenoepics could you also do an extra review so I don't overlook something?
Then I need to do a release for 9.0.0-alpha.1 as we are currently busy building everything together for the v9.0.0 release
3 participants
Closes #1470
Adds an optional
nullsoption to thecreateIndexfunction with two possible values:'distinct'and'not distinct'. If provided, adds aNULLS DISTINCTorNULLS NOT DISTINCTclause to the index creation command.Note that in the original issue I quoted from some docs saying that there were two other nulls clause possibilities:
NULLS FIRSTandNULLS LAST, but those actually are a separate option that is part of the sort order specifier, so I didn't include them (since there's no option for sort order increateIndexyet).