importccl: IMPORT INTO should support resolving default expressions referencing UDTs #61133
Labels
A-disaster-recovery
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
docs-done
docs-known-limitation
GA-blocker
T-multiregion
Comments
ajstorm
added
the
C-enhancement
ajstorm
added a commit
to ajstorm/cockroach
that referenced
this issue
Feb 28, 2021
Unblock IMPORTing into allowable multi-region tables. We only support importing into GLOBAL and REGIONAL BY TABLE tables as cockroachdb#61133 is blocking IMPORT into any tables which have columns generated using a user defined type (which covers REGIONAL BY ROW tables, as they have the crdb_region column which is generated using the crdb_internal_region type). This commit includes tests for both IMPORT and IMPORT INTO, as well as cases which illustrate cockroachdb#61133 for non-multi-region tables. Release note: None Release justification: Fixes bug in interaction between existing functionality and new multi-region feature.
ajstorm
added a commit
to ajstorm/cockroach
that referenced
this issue
Feb 28, 2021
Unblock IMPORTing into allowable multi-region tables. We only support importing into GLOBAL and REGIONAL BY TABLE tables as cockroachdb#61133 is blocking IMPORT into any tables which have columns generated using a user defined type (which covers REGIONAL BY ROW tables, as they have the crdb_region column which is generated using the crdb_internal_region type). This commit includes tests for both IMPORT and IMPORT INTO, as well as cases which illustrate cockroachdb#61133 for non-multi-region tables. One thing of note is the moving of the call to ValidateTableLocalityConfig from NewTableDesc to the startExec of createTableNode. This is intentional, and was required, as the call to NewTableDesc in IMPORT doesn't have the necessary infrastructure setup (namely, a valid transaction and the EvalContext Codec) to validate the LocalityConfig at that time. That being said, IMPORT does perform a proper validation of the full table descriptor before it's written to disk. Once it was deemed safe to remove the validation from NewTableDesc, the validate had to be added to startExec of createTableNode, as the LocalityConfig is used in there to generate a zone configuration, and we want to ensure that the LocalityConfig is valid before we do that. At this point there are no other calls to NewTableDesc which require validation of the LocalityConfig. Release note: None Release justification: Fixes bug in interaction between existing functionality and new multi-region feature.
ajstorm
added a commit
to ajstorm/cockroach
that referenced
this issue
Mar 1, 2021
Unblock IMPORTing into allowable multi-region tables. We only support importing into GLOBAL and REGIONAL BY TABLE tables as cockroachdb#61133 is blocking IMPORT into any tables which have columns generated using a user defined type (which covers REGIONAL BY ROW tables, as they have the crdb_region column which is generated using the crdb_internal_region type). This commit includes tests for both IMPORT and IMPORT INTO, as well as cases which illustrate cockroachdb#61133 for non-multi-region tables. One thing of note is the moving of the call to ValidateTableLocalityConfig from NewTableDesc to the startExec of createTableNode. This is intentional, and was required, as the call to NewTableDesc in IMPORT doesn't have the necessary infrastructure setup (namely, a valid transaction and the EvalContext Codec) to validate the LocalityConfig at that time. That being said, IMPORT does perform a proper validation of the full table descriptor before it's written to disk. Once it was deemed safe to remove the validation from NewTableDesc, the validate had to be added to startExec of createTableNode, as the LocalityConfig is used in there to generate a zone configuration, and we want to ensure that the LocalityConfig is valid before we do that. At this point there are no other calls to NewTableDesc which require validation of the LocalityConfig. Release note: None Release justification: Fixes bug in interaction between existing functionality and new multi-region feature.
ajstorm
added a commit
to ajstorm/cockroach
that referenced
this issue
Mar 2, 2021
Unblock IMPORTing into allowable multi-region tables. We only support importing into GLOBAL and REGIONAL BY TABLE tables as cockroachdb#61133 is blocking IMPORT into any tables which have columns generated using a user defined type (which covers REGIONAL BY ROW tables, as they have the crdb_region column which is generated using the crdb_internal_region type). This commit includes tests for both IMPORT and IMPORT INTO, as well as cases which illustrate cockroachdb#61133 for non-multi-region tables. One thing of note is the removing of the call to ValidateTableLocalityConfig from NewTableDesc. This is intentional, and was required, as the call to NewTableDesc in IMPORT doesn't have the necessary infrastructure setup (namely, a valid transaction and the EvalContext Codec) to validate the LocalityConfig at that time. That being said, IMPORT does perform a proper validation of the full table descriptor before it's written to disk. At this point there are no other calls to NewTableDesc which require validation of the LocalityConfig. Release note: None Release justification: Fixes bug in interaction between existing functionality and new multi-region feature.
ajstorm
added a commit
to ajstorm/cockroach
that referenced
this issue
Mar 3, 2021
Unblock IMPORTing into allowable multi-region tables. We only support importing into GLOBAL and REGIONAL BY TABLE tables as cockroachdb#61133 is blocking IMPORT into any tables which have columns generated using a user defined type (which covers REGIONAL BY ROW tables, as they have the crdb_region column which is generated using the crdb_internal_region type). This commit includes tests for both IMPORT and IMPORT INTO, as well as cases which illustrate cockroachdb#61133 for non-multi-region tables. One thing of note is the removing of the call to ValidateTableLocalityConfig from NewTableDesc. This is intentional, and was required, as the call to NewTableDesc in IMPORT doesn't have the necessary infrastructure setup (namely, a valid transaction and the EvalContext Codec) to validate the LocalityConfig at that time. That being said, IMPORT does perform a proper validation of the full table descriptor before it's written to disk. At this point there are no other calls to NewTableDesc which require validation of the LocalityConfig. Release note: None Release justification: Fixes bug in interaction between existing functionality and new multi-region feature.
craig bot
pushed a commit
that referenced
this issue
Mar 4, 2021
61192: multi-region: Unblock IMPORT into allowable multi-region tables r=otan,arulajmani,pbardea a=ajstorm Unblock IMPORTing into allowable multi-region tables. We only support importing into GLOBAL and REGIONAL BY TABLE tables as #61133 is blocking IMPORT into any tables which have columns generated using a user defined type (which covers REGIONAL BY ROW tables, as they have the crdb_region column which is generated using the crdb_internal_region type). This commit includes tests for both IMPORT and IMPORT INTO, as well as cases which illustrate #61133 for non-multi-region tables. Of note is the change to remove validation in create_table.go . This change was required as the validation at this point is not possible from within IMPORT, and it was uncovered through testing that this validation when generating the LocalityConfig was not required anyway, as we validate before writing the descriptors in all cases (i.e. after the LocalityConfig is generated, but before the descriptors are written to disk). Release note: None Release justification: Fixes bug in interaction between existing functionality and new multi-region feature. Co-authored-by: Adam Storm <[email protected]>
exalate-issue-sync
bot
added
T-multi-region
and removed
T-disaster-recovery
T-multiregion
labels
adityamaru
added a commit
to adityamaru/cockroach
that referenced
this issue
Sep 7, 2021
This change builds on the work in cockroachdb#69674 and uses the type descriptors stored on the import job during planning to construct a custom type resolver. This resolver is used when hydrating the types used by the table being imported into, and when processing default + computed columns. Informs: cockroachdb#61133 Release note (sql change): IMPORT INTO now supports UDT for default and computed columns. Release justification: fixes for high-priority or high-severity bugs in existing functionality
craig bot
pushed a commit
that referenced
this issue
Sep 8, 2021
69674: importccl: fail IMPORT INTO on concurrent type change r=arulajmani,dt a=adityamaru This change adds logic to check that the type descriptors referenced by the table being imported into have not undergone any changes (adding/dropping values) during the the course of the import job execution. This is done by matching the current type desc version against the version on the type desc that was cached in the import job during planning. If there is a mismatch, we should fail the job since the data could be referencing a value that has been dropped, leading to corrupt kv entries. Informs: #61133 Release note (bug fix): add protection to import into to guard against concurrent type changes on UDTs referenced by the target table. Release justification: fixes for high-priority or high-severity bugs in existing functionality 69881: Add Stephanie Bodoff to AUTHORS r=jlinder a=stbof Release note: None 69910: authors: add jameswsj10 to authors r=jameswsj10 a=jameswsj10 Release justification: non-production change Release note: None 69922: jobs,schedule: Annotate context with schedule ID. r=miretskiy a=miretskiy Annotate context passed to job execution with the schedule ID. Release Justification: Low danger, observability imporovement; category 4. Release Notes: None Co-authored-by: Aditya Maru <[email protected]> Co-authored-by: Stephanie Bodoff <[email protected]> Co-authored-by: jameswsj10 <[email protected]> Co-authored-by: Yevgeniy Miretskiy <[email protected]>
adityamaru
added a commit
to adityamaru/cockroach
that referenced
this issue
Sep 9, 2021
This change builds on the work in cockroachdb#69674 and uses the type descriptors stored on the import job during planning to construct a custom type resolver. This resolver is used when hydrating the types used by the table being imported into, and when processing default + computed columns. Informs: cockroachdb#61133 Release note (sql change): IMPORT INTO now supports UDT for default and computed columns. Release justification: fixes for high-priority or high-severity bugs in existing functionality Co-authored-by: Anne Zhu <[email protected]>
craig bot
pushed a commit
that referenced
this issue
Sep 10, 2021
69779: importccl: add IMPORT INTO UDT support for default+computed columns r=dt a=adityamaru This change builds on the work in #69674 and uses the type descriptors stored on the import job during planning to construct a custom type resolver. This resolver is used when hydrating the types used by the table being imported into, and when processing default + computed columns. Informs: #61133 Release note (sql change): IMPORT INTO now supports UDT for default and computed columns. Release justification: fixes for high-priority or high-severity bugs in existing functionality Co-authored-by: Aditya Maru <[email protected]>
adityamaru
added
the
release-blocker
|
Hi @adityamaru, please add branch-* labels to identify which branch(es) this release-blocker affects. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
adityamaru
added a commit
to adityamaru/cockroach
that referenced
this issue
Sep 15, 2021
This change builds on the work in cockroachdb#69674 and uses the type descriptors stored on the import job during planning to construct a custom type resolver. This resolver is used when hydrating the types used by the table being imported into, and when processing default + computed columns. Informs: cockroachdb#61133 Release note (sql change): IMPORT INTO now supports UDT for default and computed columns. Release justification: fixes for high-priority or high-severity bugs in existing functionality Co-authored-by: Anne Zhu <[email protected]>
adityamaru
added
GA-blocker
and removed
release-blocker
|
This is fixed and backported to 21.2! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently IMPORT doesn't work for tables with computed columns that use user defined types. For example, this test:
fails with:
error executing 'IMPORT INTO typ_table (i, s, b) AVRO DATA nodelocal://0/simple.ocf': pq: nodelocal://0/simple.ocf: process default and computed columns: type OID 100058 does not exist
This is also needed to allow for IMPORTing into the new multi-region REGIONAL BY ROW tables, as they use a hidden crdb_region column under the covers, which is based on the crdb_internal_region system defined type.
Epic CRDB-7074
The text was updated successfully, but these errors were encountered: