-
Notifications
You must be signed in to change notification settings - Fork 11
SPOC-283: Reallow conversions #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
mason-sharp
wants to merge
1
commit into
main
Choose a base branch
from
fix/SPOC-283/reallow-conversions
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+294
−6
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Reallow conversions
For plain sequences and explict default value sequences. Also, update to use the same naming convention for variables.
- Loading branch information
commit e171f243a1202f2e615e055324b3948b0702a4a7
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,246 @@ | ||
| /* snowflake--2.3--2.4.sql */ | ||
|
|
||
| -- complain if script is sourced in psql, rather than via ALTER EXTENSION | ||
| \echo Use "ALTER EXTENSION snowflake UPDATE TO '2.4'" to load this file. \quit | ||
|
|
||
| -- ---------------------------------------------------------------------- | ||
| -- convert_sequence_to_snowflake() | ||
| -- | ||
| -- Convert a sequence to use snowflake's nextval() for the following cases: | ||
| -- - Used in an IDENTITY constraint | ||
| -- - Used as a generated sequence for Serial or Bigserial | ||
| -- - Used with nextval() as a default value | ||
| -- - A plain sequence | ||
| -- | ||
| -- Returns the number of changes done, including column default changes, | ||
| -- and data type changes, such as changing to int8. The number does not | ||
| -- include changing the sequence itself. | ||
| -- | ||
| -- NOTES: | ||
| -- 1. Changes the data type of affected columns to bigint. | ||
| -- 2. IDENTITY ALWAYS restriction will be eased to DEFAULT. | ||
| -- ---------------------------------------------------------------------- | ||
| CREATE OR REPLACE FUNCTION snowflake.convert_sequence_to_snowflake(p_seqid regclass) | ||
| RETURNS integer | ||
| SET search_path = pg_catalog | ||
| AS $$ | ||
| DECLARE | ||
| v_objdesc record; -- contains target (reloid,attnum) value | ||
| v_identdesc record; -- sequence-related flags (attisidentity) | ||
| v_attrdef record; | ||
| v_attr record; | ||
| v_seq record; | ||
| v_cmd text; | ||
| v_num_altered integer := 0; | ||
| v_last_value bigint; | ||
| v_seqname1 text; | ||
| v_extseqname text; | ||
| v_is_serial_def boolean; | ||
| v_textstr text; | ||
| BEGIN | ||
| -- Identify the (relation,attnum) that uses this sequence as a source | ||
| -- for values. Follow the logic of the getOwnedSequences_internal. | ||
| -- | ||
| -- Complain, if such data wasn't found - incoming object may be not | ||
| -- a sequence, or sequence which is used for different purposes. | ||
| -- v_objdesc's fields: | ||
| -- heapreloid - Oid of the target relation. | ||
| -- nspname - namespace of the sequence | ||
| -- seqname - sequence name | ||
| -- attnum - number of relation's attribute that employs this sequence | ||
| SELECT INTO v_objdesc | ||
| refobjid AS heapreloid, | ||
| c.relnamespace::regnamespace::text AS nspname, | ||
| c.relname AS seqname, | ||
| refobjsubid AS attnum, | ||
| deptype | ||
| FROM pg_depend AS d JOIN pg_class AS c ON (d.objid = c.oid) | ||
| WHERE | ||
| classid = 'pg_class'::regclass AND | ||
| deptype IN ('a','i','n') AND | ||
| c.oid = p_seqid AND relkind = 'S' | ||
| ORDER BY deptype; -- prioritize (a)uto and (i)nternal before (n)ormal | ||
|
|
||
| IF (v_objdesc IS NULL) THEN | ||
| raise EXCEPTION 'Input value "%" is not a valid convertable sequence', p_seqid; | ||
| RETURN false; | ||
| END IF; | ||
|
|
||
| -- ---- | ||
| -- We are looking for column defaults that use the requested | ||
| -- sequence and the function nextval(). Because pg_get_expr() | ||
| -- omits the schemaname of the sequence if it is "public" we | ||
| -- need to be prepared for a schema qualified and unqualified | ||
| -- name here. | ||
| -- ---- | ||
| SELECT INTO v_seqname1 | ||
| quote_ident(v_objdesc.seqname); | ||
| SELECT INTO v_extseqname | ||
| quote_ident(v_objdesc.nspname) || '.' || quote_ident(v_objdesc.seqname); | ||
|
|
||
| IF v_objdesc.deptype IN ('a','i') THEN | ||
| -- Handle IDENTITY case and SERIAL/BIGSERIAL case | ||
|
|
||
| SELECT INTO v_identdesc | ||
| (attidentity = 'a' OR attidentity = 'd') AS is_identity, | ||
| c.relnamespace::regnamespace::text AS nspname, | ||
| c.relname AS relname, | ||
| a.attname AS attname | ||
| FROM pg_attribute a JOIN pg_class c ON (c.oid = a.attrelid) | ||
| WHERE a.attrelid = v_objdesc.heapreloid AND a.attnum = v_objdesc.attnum; | ||
|
|
||
| IF (v_identdesc.is_identity) THEN | ||
| UPDATE pg_attribute SET attidentity = '' | ||
| WHERE attrelid = v_objdesc.heapreloid AND attnum = v_objdesc.attnum; | ||
| RAISE NOTICE | ||
| 'Update pg_attribute: reset attidentity value for table %, column %', | ||
| v_objdesc.heapreloid::regclass, v_identdesc.attname; | ||
| ELSE | ||
|
|
||
| -- Extract DEFAULT definition for the column and conver it into | ||
| -- a readable string representation. | ||
| SELECT INTO v_textstr | ||
| pg_get_expr(ad.adbin, ad.adrelid, true) | ||
| FROM pg_attrdef ad | ||
| WHERE adrelid = v_objdesc.heapreloid AND adnum = v_objdesc.attnum; | ||
|
|
||
| -- Check that the DEFAULT expression contains input sequence in a form | ||
| -- as related to the serial and bigserial type. | ||
| SELECT INTO v_is_serial_def | ||
| CASE WHEN | ||
| v_textstr = 'nextval(' || quote_literal(v_seqname1) || '::regclass)' OR | ||
| v_textstr = 'nextval(' || quote_literal(v_extseqname) || '::regclass)' | ||
| THEN true ELSE false END; | ||
|
|
||
| -- If there is another DEFAULT expression already set for this column, we | ||
| -- are not so bold to remove it, just complain, | ||
| IF (NOT v_is_serial_def) THEN | ||
| raise EXCEPTION | ||
| 'definition of DEFAULT value for column "%" of relation "%" does not correspond serial or bigserial type: "%"', | ||
| v_identdesc.attname, v_identdesc.relname, v_textstr; | ||
| END IF; | ||
| END IF; | ||
|
|
||
| -- ---- | ||
| -- If the attribute type is not bigint, we change it | ||
| -- ---- | ||
| v_num_altered = | ||
| snowflake.convert_column_to_int8(v_objdesc.heapreloid::regclass, | ||
| v_objdesc.attnum::smallint); | ||
|
|
||
| -- ---- | ||
| -- Now we can change the default to snowflake.nextval() | ||
| -- ---- | ||
| v_cmd = 'ALTER TABLE ' || | ||
| quote_ident(v_identdesc.nspname) || '.' || | ||
| quote_ident(v_identdesc.relname) || | ||
| ' ALTER COLUMN ' || quote_ident(v_identdesc.attname) || | ||
| ' SET DEFAULT snowflake.nextval(' || | ||
| quote_literal( | ||
| quote_ident(v_objdesc.nspname) || '.' || | ||
| quote_ident(v_objdesc.seqname) | ||
| ) || '::regclass)'; | ||
| RAISE NOTICE 'EXECUTE %', v_cmd; | ||
| EXECUTE v_cmd; | ||
| ELSE | ||
| -- Look for cases where the sequence is used as an explicit default value | ||
|
|
||
| FOR v_attrdef IN | ||
| WITH AD AS ( | ||
| SELECT AD.*, | ||
| pg_get_expr(AD.adbin, AD.adrelid, true) adstr | ||
| FROM pg_attrdef AD | ||
| ) | ||
| SELECT * FROM AD | ||
| WHERE AD.adstr = 'nextval(' || quote_literal(v_seqname1) || '::regclass)' | ||
| OR AD.adstr = 'nextval(' || quote_literal(v_extseqname) || '::regclass)' | ||
| LOOP | ||
| -- ---- | ||
| -- Get the attribute definition | ||
| -- ---- | ||
| SELECT * INTO v_attr | ||
| FROM pg_namespace N | ||
| JOIN pg_class C | ||
| ON N.oid = C.relnamespace | ||
| JOIN pg_attribute A | ||
| ON C.oid = A.attrelid | ||
| WHERE A.attrelid = v_attrdef.adrelid | ||
| AND A.attnum = v_attrdef.adnum; | ||
|
|
||
| IF NOT FOUND THEN | ||
| RAISE EXCEPTION 'Attribute for % not found', v_attrdef.adstr; | ||
| END IF; | ||
|
|
||
| -- ---- | ||
| -- Get the sequence definition | ||
| -- ---- | ||
| SELECT * INTO v_seq | ||
| FROM pg_namespace N | ||
| JOIN pg_class C | ||
| ON N.oid = C.relnamespace | ||
| WHERE C.oid = p_seqid; | ||
|
|
||
| IF NOT FOUND THEN | ||
| RAISE EXCEPTION 'Sequence with Oid % not found', p_seqid; | ||
| END IF; | ||
|
|
||
| -- ---- | ||
| -- If the attribute type is not bigint, we change it | ||
| -- ---- | ||
| v_num_altered = v_num_altered + | ||
| snowflake.convert_column_to_int8(v_attr.attrelid, v_attr.attnum); | ||
|
|
||
| -- ---- | ||
| -- Now we can change the default to snowflake.nextval() | ||
| -- ---- | ||
| v_cmd = 'ALTER TABLE ' || | ||
| quote_ident(v_attr.nspname) || '.' || | ||
| quote_ident(v_attr.relname) || | ||
| ' ALTER COLUMN ' || | ||
| quote_ident(v_attr.attname) || | ||
| ' SET DEFAULT snowflake.nextval(' || | ||
| quote_literal( | ||
| quote_ident(v_seq.nspname) || '.' || | ||
| quote_ident(v_seq.relname) | ||
| ) || | ||
| '::regclass)'; | ||
| RAISE NOTICE 'EXECUTE %', v_cmd; | ||
| EXECUTE v_cmd; | ||
|
|
||
| v_num_altered = v_num_altered + 1; | ||
| END LOOP; | ||
| END IF; | ||
|
|
||
| -- Note: for plain sequences not associated with columns, we still | ||
| -- fall through to here so snowflake can be used, to support | ||
| -- sequences used explicitly by applications | ||
|
|
||
| -- ---- | ||
| -- Finally we need to change the sequence itself to settings that | ||
| -- prevent pg_catalog.nextval() from working. We do this by setting | ||
| -- the sequence's MAXVAL to its current last_value + 1, then invoke | ||
| -- our own nextval() function to bump it. | ||
| -- ---- | ||
| v_cmd = 'SELECT last_value FROM ' || | ||
| pg_catalog.quote_ident(N.nspname) || '.' || | ||
| pg_catalog.quote_ident(C.relname) | ||
| FROM pg_catalog.pg_class C | ||
| JOIN pg_catalog.pg_namespace N ON N.oid = C.relnamespace | ||
| WHERE C.oid = p_seqid; | ||
| EXECUTE v_cmd INTO v_last_value; | ||
|
|
||
| v_cmd = 'ALTER SEQUENCE ' || | ||
| pg_catalog.quote_ident(N.nspname) || '.' || | ||
| pg_catalog.quote_ident(C.relname) || | ||
| ' NO CYCLE MAXVALUE ' || | ||
| v_last_value + 1 | ||
| FROM pg_catalog.pg_class C | ||
| JOIN pg_catalog.pg_namespace N ON N.oid = C.relnamespace | ||
| WHERE C.oid = p_seqid; | ||
| RAISE NOTICE '%', v_cmd; | ||
| EXECUTE v_cmd; | ||
|
|
||
| PERFORM snowflake.nextval(p_seqid); | ||
| RETURN v_num_altered; | ||
| END; | ||
| $$ LANGUAGE plpgsql; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| # snowflake extension | ||
| comment = 'Snowflake style IDs for PostgreSQL' | ||
| default_version = '2.3' | ||
| default_version = '2.4' | ||
| module_pathname = '$libdir/snowflake' | ||
| relocatable = false | ||
| schema = snowflake |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can patch version 2.3 directly, if it hasn't been released yet. At least, it would be easier to see the changes.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! We did create a tag though. Let me check with a couple of people.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will go with 2.4 after all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason? Someone officially released 2.3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The v2.3 has been sitting out there a while. No big deal to just go to 2.4 instead of messing someone up who is using the repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say we should be more strict about release cuts in this case. A new version of an extension is always a massive pain for developers and a source of multiple bugs.
If you still insist on the new version, please don't forget to attach snowflake--2.4.sql.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need for a snowflake--2.4.sql file, there is an ugrade path file to 2.4. CREATE EXTENSION will execute snowflake--2.3.sql and then snowflake--2.3--2.4.sql.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please correct me if I'm wrong, but we have versions like snowflake--2.3 and spock--5.0.0.sql. This is a common practice. The reason behind it is that the update scripts can be messy for developers and prone to errors. Furthermore, users generally prefer to see the complete script rather than just the transient parts.