-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
sql: add support for the refcursor data type #111560
Labels
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Comments
DrewKimball
added
the
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
label
Oct 2, 2023
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 2, 2023
…creation It is necessary to check the cluster version when building an expression that references a newly-added type, in order to ensure that all nodes support that type. Previously, these checks were omitted for casts in the vectorized engine, expressions for partial index predicates and check constraints, function parameters and return types, and user-defined composite types. This patch adds version-checking for function/procedure parameters and return types, as well as for user-defined types. It also augments the type-checking logic for casts and type annotations with a version check; this handles the remaining cases. The execution-time checks for cast expressions are left untouched, just in case the new type-checking logic misses important cases. For now, these changes only apply to the `PG_LSN` type, which will be new in 23.2. A future commit will add support for `REFCURSOR`, and will need to use the same checks. Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 2, 2023
This patch adds support for the `REFCURSOR` type. It is a special type used to hold the name of a cursor for PLpgSQL, although it can be used in other contexts (like function return types or column types). `REFCURSOR` is a string under the hood, and behaves like the `TEXT` data type in most cases. `REFCURSOR` has no casts in the `pg_cast` table; instead, all of its casts are the "IO" string casts. That means that there is an assignment cast from every type to `REFCURSOR, and an explicit cast from `REFCURSOR` to every other type (except other string types, which are again assignment). See `ContextOriginAutomaticIOConversion` in `cast.go` for more info. This patch also adds `REFCURSOR` to the checks from the previous commit, so that statements with `REFCURSOR` are disallowed unless all nodes are running v23.2. Fixes cockroachdb#111560 Release note (sql change): Added support for the `REFCURSOR` data type. `REFCURSOR` is a special string type that is used to handle cursors. PLpgSQL cursor declarations are required to use a variable of type `REFCURSOR`, and the name of a cursor can be passed to and from a PLpgSQL function or procedure.
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 3, 2023
…creation It is necessary to check the cluster version when building an expression that references a newly-added type, in order to ensure that all nodes support that type. Previously, these checks were omitted for casts in the vectorized engine, expressions for partial index predicates and check constraints, function parameters and return types, and user-defined composite types. This patch adds version-checking for function/procedure parameters and return types, as well as for user-defined types. It also augments the type-checking logic for casts and type annotations with a version check; this handles the remaining cases. The execution-time checks for cast expressions are left untouched, just in case the new type-checking logic misses important cases. For now, these changes only apply to the `PG_LSN` type, which will be new in 23.2. A future commit will add support for `REFCURSOR`, and will need to use the same checks. Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 3, 2023
This patch adds support for the `REFCURSOR` type. It is a special type used to hold the name of a cursor for PLpgSQL, although it can be used in other contexts (like function return types or column types). `REFCURSOR` is a string under the hood, and behaves like the `TEXT` data type in most cases. `REFCURSOR` has no casts in the `pg_cast` table; instead, all of its casts are the "IO" string casts. That means that there is an assignment cast from every type to `REFCURSOR, and an explicit cast from `REFCURSOR` to every other type (except other string types, which are again assignment). See `ContextOriginAutomaticIOConversion` in `cast.go` for more info. This patch also adds `REFCURSOR` to the checks from the previous commit, so that statements with `REFCURSOR` are disallowed unless all nodes are running v23.2. Fixes cockroachdb#111560 Release note (sql change): Added support for the `REFCURSOR` data type. `REFCURSOR` is a special string type that is used to handle cursors. PLpgSQL cursor declarations are required to use a variable of type `REFCURSOR`, and the name of a cursor can be passed to and from a PLpgSQL function or procedure.
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 4, 2023
…creation It is necessary to check the cluster version when building an expression that references a newly-added type, in order to ensure that all nodes support that type. Previously, these checks were omitted for casts in the vectorized engine, expressions for partial index predicates and check constraints, function parameters and return types, and user-defined composite types. This patch adds version-checking for function/procedure parameters and return types, as well as for user-defined types. It also augments the type-checking logic for casts and type annotations with a version check; this handles the remaining cases. The execution-time checks for cast expressions are left untouched, just in case the new type-checking logic misses important cases. For now, these changes only apply to the `PG_LSN` type, which will be new in 23.2. A future commit will add support for `REFCURSOR`, and will need to use the same checks. Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 4, 2023
This patch adds support for the `REFCURSOR` type. It is a special type used to hold the name of a cursor for PLpgSQL, although it can be used in other contexts (like function return types or column types). `REFCURSOR` is a string under the hood, and behaves like the `TEXT` data type in most cases. `REFCURSOR` has no casts in the `pg_cast` table; instead, all of its casts are the "IO" string casts. That means that there is an assignment cast from every type to `REFCURSOR`, and an explicit cast from `REFCURSOR` to every other type (except other string types, which are again assignment). See `ContextOriginAutomaticIOConversion` in `cast.go` for more info. This patch also adds `REFCURSOR` to the checks from the previous commit, so that statements with `REFCURSOR` are disallowed unless all nodes are running v23.2. Fixes cockroachdb#111560 Release note (sql change): Added support for the `REFCURSOR` data type. `REFCURSOR` is a special string type that is used to handle cursors. PLpgSQL cursor declarations are required to use a variable of type `REFCURSOR`, and the name of a cursor can be passed to and from a PLpgSQL function or procedure.
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 4, 2023
…creation It is necessary to check the cluster version when building an expression that references a newly-added type, in order to ensure that all nodes support that type. Previously, these checks were omitted for casts in the vectorized engine, expressions for partial index predicates and check constraints, function parameters and return types, and user-defined composite types. This patch adds version-checking for function/procedure parameters and return types, as well as for user-defined types. It also augments the type-checking logic for casts and type annotations with a version check; this handles the remaining cases. The execution-time checks for cast expressions are left untouched, just in case the new type-checking logic misses important cases. For now, these changes only apply to the `PG_LSN` type, which will be new in 23.2. A future commit will add support for `REFCURSOR`, and will need to use the same checks. Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 4, 2023
This patch adds support for the `REFCURSOR` type. It is a special type used to hold the name of a cursor for PLpgSQL, although it can be used in other contexts (like function return types or column types). `REFCURSOR` is a string under the hood, and behaves like the `TEXT` data type in most cases. `REFCURSOR` has no casts in the `pg_cast` table; instead, all of its casts are the "IO" string casts. That means that there is an assignment cast from every type to `REFCURSOR`, and an explicit cast from `REFCURSOR` to every other type (except other string types, which are again assignment). See `ContextOriginAutomaticIOConversion` in `cast.go` for more info. This patch also adds `REFCURSOR` to the checks from the previous commit, so that statements with `REFCURSOR` are disallowed unless all nodes are running v23.2. Fixes cockroachdb#111560 Release note (sql change): Added support for the `REFCURSOR` data type. `REFCURSOR` is a special string type that is used to handle cursors. PLpgSQL cursor declarations are required to use a variable of type `REFCURSOR`, and the name of a cursor can be passed to and from a PLpgSQL function or procedure.
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 5, 2023
…creation It is necessary to check the cluster version when building an expression that references a newly-added type, in order to ensure that all nodes support that type. Previously, these checks were omitted for casts in the vectorized engine, expressions for partial index predicates and check constraints, function parameters and return types, and user-defined composite types. This patch adds version-checking for function/procedure parameters and return types, as well as for user-defined types. It also augments the type-checking logic for casts and type annotations with a version check; this handles the remaining cases. The execution-time checks for cast expressions are left untouched, just in case the new type-checking logic misses important cases. For now, these changes only apply to the `PG_LSN` type, which will be new in 23.2. A future commit will add support for `REFCURSOR`, and will need to use the same checks. Informs cockroachdb#111560 Release note: None
craig bot
pushed a commit
that referenced
this issue
Oct 5, 2023
111392: sql: add refcursor type r=DrewKimball a=DrewKimball #### types: remove pg_lsn from postgresPredefinedTypeIssues This patch removes pg_lsn from the list of predefined types in postgres that aren't yet supported, since it can now be parsed. Informs #105130 Release note: None #### sql: check unsupported types during schema changes and type/function creation It is necessary to check the cluster version when building an expression that references a newly-added type, in order to ensure that all nodes support that type. Previously, these checks were omitted for casts in the vectorized engine, expressions for partial index predicates and check constraints, function parameters and return types, and user-defined composite types. This patch adds version-checking for function/procedure parameters and return types, as well as for user-defined types. It also augments the type-checking logic for casts and type annotations with a version check; this handles the remaining cases. The execution-time checks for cast expressions are left untouched, just in case the new type-checking logic misses important cases. For now, these changes only apply to the `PG_LSN` type, which will be new in 23.2. A future commit will add support for `REFCURSOR`, and will need to use the same checks. Informs #111560 Release note: None #### sql: add support for REFCURSOR data type This patch adds support for the `REFCURSOR` type. It is a special type used to hold the name of a cursor for PLpgSQL, although it can be used in other contexts (like function return types or column types). `REFCURSOR` is a string under the hood, and behaves like the `TEXT` data type in most cases. `REFCURSOR` has no casts in the `pg_cast` table; instead, all of its casts are the "IO" string casts. That means that there is an assignment cast from every type to `REFCURSOR`, and an explicit cast from `REFCURSOR` to every other type (except other string types, which are again assignment). See `ContextOriginAutomaticIOConversion` in `cast.go` for more info. This patch also adds `REFCURSOR` to the checks from the previous commit, so that statements with `REFCURSOR` are disallowed unless all nodes are running v23.2. Fixes #111560 Release note (sql change): Added support for the `REFCURSOR` data type. `REFCURSOR` is a special string type that is used to handle cursors. PLpgSQL cursor declarations are required to use a variable of type `REFCURSOR`, and the name of a cursor can be passed to and from a PLpgSQL function or procedure. Co-authored-by: Drew Kimball <[email protected]>
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 5, 2023
The initial implementation for the `REFCURSOR` data type included assignment casts from and explicit casts to every other type. In postgres, `REFCURSOR` has only the following valid casts: 1. Explicit casts from each string type. 2. Assignment casts to each string type. This patch aligns the casts for `REFCURSOR` with those of postgres, and adds according tests. Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 6, 2023
The initial implementation for the `REFCURSOR` data type included assignment casts from and explicit casts to every other type. In postgres, `REFCURSOR` has only the following valid casts: 1. Explicit casts from each string type. 2. Assignment casts to each string type. This patch aligns the casts for `REFCURSOR` with those of postgres, and adds according tests. Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 6, 2023
Previously, REFCURSOR shared a type family with the string-like types. However, since it can't be implicitly casted to and from string types in all cases, it needs to have its own family (since types within a family must be compatible). This patch adds the new `RefCursorFamily`, and follows the example of the PG_LSN type in plumbing it throughout the SQL layer. Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 6, 2023
This patch adds the new `REFCURSOR` data type to the list of types that a string constant can have. This will allow constant values to avoid an explicit cast to `REFCURSOR` in cases when the context expects a `REFCURSOR`, e.g. inserting into a `REFCURSOR` column or calling a routine with a `REFCURSOR` parameter. Example: ``` CREATE FUNCTION f(curs REFCURSOR) RETURNS REFCURSOR AS $$ SELECT curs; $$ LANGUAGE SQL; -- This cast was necessary before: SELECT f('foo'::REFCURSOR); -- Now this works: SELECT f('foo'); ``` Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 6, 2023
The `REFCURSOR` data type does not support comparisons apart from a few edge cases like `'foo'::REFCURSOR IS NULL`. Example: ``` postgres=# SELECT 'foo'::REFCURSOR = 'foo'::REFCURSOR; ERROR: operator does not exist: refcursor = refcursor ``` The previous commit that added the `RefCursorFamily` type family already effectively removed support for most comparisons; this commit also prevents the `IS / IS DISTINCT FROM` variants from being used on a `REFCURSOR` expression. Note that postgres supports a few cases with `NULL` as mentioned above. However, this case seems unimportant, so I leave it as a TODO for now. This commit also adds tests for various comparisons involving `REFCURSOR`. Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 6, 2023
The initial implementation for the `REFCURSOR` data type included assignment casts from and explicit casts to every other type. In postgres, `REFCURSOR` has only the following valid casts: 1. Explicit casts from each string type. 2. Assignment casts to each string type. This patch aligns the casts for `REFCURSOR` with those of postgres, and adds according tests. Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 6, 2023
Previously, REFCURSOR shared a type family with the string-like types. However, since it can't be implicitly casted to and from string types in all cases, it needs to have its own family (since types within a family must be compatible). This patch adds the new `RefCursorFamily`, and follows the example of the PG_LSN type in plumbing it throughout the SQL layer. Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 6, 2023
This patch adds the new `REFCURSOR` data type to the list of types that a string constant can have. This will allow constant values to avoid an explicit cast to `REFCURSOR` in cases when the context expects a `REFCURSOR`, e.g. inserting into a `REFCURSOR` column or calling a routine with a `REFCURSOR` parameter. Example: ``` CREATE FUNCTION f(curs REFCURSOR) RETURNS REFCURSOR AS $$ SELECT curs; $$ LANGUAGE SQL; -- This cast was necessary before: SELECT f('foo'::REFCURSOR); -- Now this works: SELECT f('foo'); ``` Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 6, 2023
The `REFCURSOR` data type does not support comparisons apart from a few edge cases like `'foo'::REFCURSOR IS NULL`. Example: ``` postgres=# SELECT 'foo'::REFCURSOR = 'foo'::REFCURSOR; ERROR: operator does not exist: refcursor = refcursor ``` The previous commit that added the `RefCursorFamily` type family already effectively removed support for most comparisons; this commit also prevents the `IS / IS DISTINCT FROM` variants from being used on a `REFCURSOR` expression. Note that postgres supports a few cases with `NULL` as mentioned above. However, this case seems unimportant, so I leave it as a TODO for now. This commit also adds tests for various comparisons involving `REFCURSOR`. Informs cockroachdb#111560 Release note: None
craig bot
pushed a commit
that referenced
this issue
Oct 6, 2023
111884: cast: remove invalid casts for REFCURSOR r=DrewKimball a=DrewKimball #### cast: remove invalid casts for REFCURSOR The initial implementation for the `REFCURSOR` data type included assignment casts from and explicit casts to every other type. In postgres, `REFCURSOR` has only the following valid casts: 1. Explicit casts from each string type. 2. Assignment casts to each string type. This patch aligns the casts for `REFCURSOR` with those of postgres, and adds according tests. Informs #111560 Release note: None 111923: kv: dequeue request from lock table wait-queues on scan error r=nvanbenschoten a=nvanbenschoten Informs #111352. Informs #111530. Informs #111564. Informs #111893. In 8205b43, we added a case to the lock table where a request's initial scan could throw an error. This was not being handled properly if the request had already entered any other lock wait-queues. In these cases, the request's entries in those wait-queues would be abandoned and the locks would get stuck. This commit fixes that issue by dequeuing the request from the lock table when throwing an error from ScanAndEnqueue. This was one of the causes of the recent kvnemesis instability, but we believe that there is at least one other issue that is still causing timeouts. Release note: None Co-authored-by: Drew Kimball <[email protected]> Co-authored-by: Nathan VanBenschoten <[email protected]>
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 7, 2023
This patch removes the case statement that checks whether a cursor name is NULL before calling into `plpgsql_gen_cursor_name` because the builtin already checks whether the argument is NULL, and `REFCURSOR` will (correctly) not support comparisons after we add a new type family for it. The return type for the call to `plpgsql_gen_cursor_name` is also changed from `String` to `Refcursor`. Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 7, 2023
This patch adds the new `REFCURSOR` data type to the list of types that a string constant can have. This will allow constant values to avoid an explicit cast to `REFCURSOR` in cases when the context expects a `REFCURSOR`, e.g. inserting into a `REFCURSOR` column or calling a routine with a `REFCURSOR` parameter. Example: ``` CREATE FUNCTION f(curs REFCURSOR) RETURNS REFCURSOR AS $$ SELECT curs; $$ LANGUAGE SQL; -- This cast was necessary before: SELECT f('foo'::REFCURSOR); -- Now this works: SELECT f('foo'); ``` Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 7, 2023
Previously, REFCURSOR shared a type family with the string-like types. However, since it can't be implicitly casted to and from string types in all cases, it needs to have its own family (since types within a family must be compatible). This patch adds the new `RefCursorFamily`, and follows the example of the PG_LSN type in plumbing it throughout the SQL layer. Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 7, 2023
The `REFCURSOR` data type does not support comparisons apart from a few edge cases like `'foo'::REFCURSOR IS NULL`. Example: ``` postgres=# SELECT 'foo'::REFCURSOR = 'foo'::REFCURSOR; ERROR: operator does not exist: refcursor = refcursor ``` The previous commit that added the `RefCursorFamily` type family already effectively removed support for most comparisons; this commit also prevents the `IS / IS DISTINCT FROM` variants from being used on a `REFCURSOR` expression. Note that postgres supports a few cases with `NULL` as mentioned above. However, this case seems unimportant, so I leave it as a TODO for now. This commit also adds tests for various comparisons involving `REFCURSOR`. Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 7, 2023
Previously, REFCURSOR shared a type family with the string-like types. However, since it can't be implicitly casted to and from string types in all cases, it needs to have its own family (since types within a family must be compatible). This patch adds the new `RefCursorFamily`, and follows the example of the PG_LSN type in plumbing it throughout the SQL layer. Informs cockroachdb#111560 Release note: None
DrewKimball
added a commit
to DrewKimball/cockroach
that referenced
this issue
Oct 7, 2023
The `REFCURSOR` data type does not support comparisons apart from a few edge cases like `'foo'::REFCURSOR IS NULL`. Example: ``` postgres=# SELECT 'foo'::REFCURSOR = 'foo'::REFCURSOR; ERROR: operator does not exist: refcursor = refcursor ``` The previous commit that added the `RefCursorFamily` type family already effectively removed support for most comparisons; this commit also prevents the `IS / IS DISTINCT FROM` variants from being used on a `REFCURSOR` expression. Note that postgres supports a few cases with `NULL` as mentioned above. However, this case seems unimportant, so I leave it as a TODO for now. This commit also adds tests for various comparisons involving `REFCURSOR`. Informs cockroachdb#111560 Release note: None
craig bot
pushed a commit
that referenced
this issue
Oct 7, 2023
111906: types: give REFCURSOR its own type family r=DrewKimball a=DrewKimball #### plpgsql: prepare cursor name generation for REFCURSOR type family This patch removes the case statement that checks whether a cursor name is NULL before calling into `plpgsql_gen_cursor_name` because the builtin already checks whether the argument is NULL, and `REFCURSOR` will (correctly) not support comparisons after we add a new type family for it. The return type for the call to `plpgsql_gen_cursor_name` is also changed from `String` to `Refcursor`. Informs #111560 Release note: None #### tree: add REFCURSOR to the list of parsable string types This patch adds the new `REFCURSOR` data type to the list of types that a string constant can have. This will allow constant values to avoid an explicit cast to `REFCURSOR` in cases when the context expects a `REFCURSOR`, e.g. inserting into a `REFCURSOR` column or calling a routine with a `REFCURSOR` parameter. Example: ``` CREATE FUNCTION f(curs REFCURSOR) RETURNS REFCURSOR AS $$ SELECT curs; $$ LANGUAGE SQL; -- This cast was necessary before: SELECT f('foo'::REFCURSOR); -- Now this works: SELECT f('foo'); ``` Informs #111560 Release note: None #### types: give REFCURSOR its own type family Previously, REFCURSOR shared a type family with the string-like types. However, since it can't be implicitly casted to and from string types in all cases, it needs to have its own family (since types within a family must be compatible). This patch adds the new `RefCursorFamily`, and follows the example of the PG_LSN type in plumbing it throughout the SQL layer. Informs #111560 Release note: None #### tree: disallow and test comparison operators for REFCURSOR The `REFCURSOR` data type does not support comparisons apart from a few edge cases like `'foo'::REFCURSOR IS NULL`. Example: ``` postgres=# SELECT 'foo'::REFCURSOR = 'foo'::REFCURSOR; ERROR: operator does not exist: refcursor = refcursor ``` The previous commit that added the `RefCursorFamily` type family already effectively removed support for most comparisons; this commit also prevents the `IS / IS DISTINCT FROM` variants from being used on a `REFCURSOR` expression. Note that postgres supports a few cases with `NULL` as mentioned above. However, this case seems unimportant, so I leave it as a TODO for now. This commit also adds tests for various comparisons involving `REFCURSOR`. Informs #111560 Release note: None Co-authored-by: Drew Kimball <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
PG has a special type
REFCURSOR
, which is basically a string, but is used to hold the name of a PLpgSQL cursor. While it's generally used as a variable in a PLpgSQL routine, it can also be used in other contexts (e.g. function parameter or return type, table column type etc.). The type isn't recognized by previous versions of CRDB, so we'll have to add validation to ensure that statements that useREFCURSOR
fail until the cluster is fully upgraded to 23.2.Jira issue: CRDB-31968
The text was updated successfully, but these errors were encountered: