Skip to content
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.

Sign up for GitHub

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

Jump to bottom

sql: consider disabling cross database type references #49809

Closed
rohany opened this issue Jun 2, 2020 · 0 comments · Fixed by #49841
Closed

sql: consider disabling cross database type references #49809

rohany opened this issue Jun 2, 2020 · 0 comments · Fixed by #49841
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@rohany
Copy link
Contributor

rohany commented Jun 2, 2020

Now that we are adding user defined schemas, there aren't many reasons to provide cross database object references. In particular, cross database type references can lead to problems around cockroach dump, as well as DROP TYPE affecting data in other databases. It seems like we should disallow creating tables with types in other databases.

Relates to #47765.
@rohany rohany added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Jun 2, 2020
@rohany rohany mentioned this issue Jun 2, 2020
Closed
35 tasks
@rohany rohany mentioned this issue Jun 3, 2020
Merged
rohany added a commit to rohany/cockroach that referenced this issue Jun 4, 2020
@rohany
sql: disallow cross database type references
ad58590 
Fixes cockroachdb#49809.

This PR disallows using types from other databases in tables. This makes
certain behavior (like `DROP TYPE`) more predictable in their effects,
as well as unblocking some work for supporting user defined types in
`cockroach dump`.

Release note (sql change): Referencing types across databases has been
disabled.
rohany added a commit to rohany/cockroach that referenced this issue Jun 4, 2020
@rohany
sql: disallow cross database type references
4c1147a 
Fixes cockroachdb#49809.

This PR disallows using types from other databases in tables. This makes
certain behavior (like `DROP TYPE`) more predictable in their effects,
as well as unblocking some work for supporting user defined types in
`cockroach dump`.

Release note (sql change): Referencing types across databases has been
disabled.
craig bot pushed a commit that referenced this issue Jun 8, 2020
@jordanlewis @rohany
@yuzefovich @rytaft
Merge #49728 #49841 #49919 #49958 #49961
a966fd4 
49728: execgen: add generic inliner r=jordanlewis a=jordanlewis

This commit adds a new directive to execgen:

// execgen:inline

This directive causes a function that's annotated with it to be inlined
into all of its callers via AST transformation. This kind of inlining is
smarter than before - it permits arguments with names different from the
names of the function's formal parameters, for example.

This commit also changes the functions in distinct_tmpl to use this
directive instead of the manual templating method.

I think this is the only easy function to change. The rest have, at the
simple level of difficulty, static template-time parameters, and at a
harder level of difficulty, type parameters. Improving these cases
holistically should come next.

Release note: None

49841: sql: disallow cross database type references r=otan a=rohany

Fixes #49809.

This PR disallows using types from other databases in tables. This makes
certain behavior (like `DROP TYPE`) more predictable in their effects,
as well as unblocking some work for supporting user defined types in
`cockroach dump`.

Release note (sql change): Referencing types across databases has been
disabled.

49919: sql: use nicer apd.Decimal.SetInt64 in the code base r=yuzefovich a=yuzefovich

This commit is automatic replacement of `apd.Decimal.SetFinite((.*), 0)`
into a nicer `apd.Decimal.SetInt64($1)` which are equivalent.

Release note: None

49958: jobs: rename Resume to Unpause r=spaskob a=spaskob

Resume verb is already used to for the action of starting a job
after adoption on a node. Here instead Resume is the opposite
action of pausing a job.

Release note: none.

49961: opt: fix error caused by recursive CTE with zero rows on left side r=rytaft a=rytaft

Prior to this commit, a recursive CTE in which the cardinality of
the left side of the `UNION ALL` expression was zero would cause an
error in the statistics code, "estimated row count must be non-zero".
This was happening because the cardinality of the recursive CTE binding
props was set to be non-zero, but the row count, which came from the
left side expression, was not updated accordingly. The stats code only
allows the row count to be zero if the cardinality is also zero.

This commit fixes the problem by setting the row count of the binding
props to 1 if the estimated row count of the left side is less than 1.

Fixes #49911

Release note (bug fix): Fixed an internal planning error that occured
for recursive CTEs (WITH RECURSIVE expressions) in which the left side
of the UNION ALL query used in the CTE definition produced zero rows.

Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Spas Bojanov <spas@cockroachlabs.com>
Co-authored-by: Rebecca Taft <becca@cockroachlabs.com>
@craig craig bot closed this as completed in 9a4aae5 Jun 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: disallow cross database type references rohany/cockroach
1 participant
@rohany