opt: fix error in case of NULL arguments to AddGeometryColumn #50992
Conversation
This file contains 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 commit fixes an error that occurred when AddGeometryColumn was called with NULL arguments. The error was caused by an assertion that the output of TypeCheck was a tree.FuncExpr when in fact it was tree.DNull. This commit fixes the error by adding an explicit check for tree.DNull after calling TypeCheck. Fixes cockroachdb#50296 Release note (bug fix): Fixed an internal error that occurred when AddGeometryColumn was called with NULL arguments. This now results in a no-op and returns NULL.
|
This change is |
|
TFTR! bors r+ |
|
bors canceled the batch because of the failed CLA check on another PR. bors r+ |
Build succeeded |
rytaft
added a commit
to rytaft/cockroach
that referenced
this pull request
Jul 9, 2020
This commit fixes an error that occurred when AddGeometryColumn was called with NULL arguments that were cast to the type specified by the function signature. cockroachdb#50992 already fixed the case when AddGeometryColumn was called with bare NULL arguments, since those were detected by TypeCheck. TypeCheck does not detect NULL arguments if they are cast to the correct type. This commit fixes the error by adding an explicit check in the optbuilder that each argument is not null before calling the SQLFn of the AddGeometryColumn overload. Informs cockroachdb#50296 Release note (bug fix): Fixed an internal error that occurred when AddGeometryColumn was called with NULL arguments. This now results in a no-op and returns NULL.
craig bot
pushed a commit
that referenced
this pull request
Jul 9, 2020
50924: cli: add commands for managing statement diagnostics r=RaduBerinde a=RaduBerinde This change adds a `statement-diag` command, with the following subcommands: ``` list list available bundles and outstanding activation requests download download statement diagnostics bundle into a zip file delete delete a statement diagnostics bundle cancel cancel an outstanding activation request ``` Fixes #48597. Release note (cli change): A new set of `statement-diag` CLI commands that can be used to manage statement diagnostics. 51065: importccl: fix target column ordering bug for PGDUMP import r=mjibson a=Anzoteh96 The current implementation assumes that the target columns of a PGDUMP query is the same as how they are created in the case where target columns are declared in PGDUMP file. This PR addresses it by detecting the target columns in the PGDUMP statement itself if this is the case. In addition, given that the target columns may not be well-determined at the formation of a new `DatumRowConverter`, so the check of unsupported default column expression is also moved to the `DatumRowConverter.Row()` function. Fixed #51159 Release note: None 51121: sql: move parallelize scans control in the execbuilder r=RaduBerinde a=RaduBerinde Parallel scans refers to disabling scan batch limits, which allows the distsender to issue requests to multiple ranges in parallel. This is only safe to use when there is a known upper bound for the number of results. Currently we plumb maxResults to the scanNode and TableReader, and each execution component runs similar logic to decide whether to parallelize. This change cleans this up by centralizing this decision inside the execbuilder. In the future, we may want the coster to be aware of this parallelization as well. For simplicity, we drop the cluster setting that controls this (it was added for fear of problems but it has been on by default for a long time). Release note: None 51163: jobs: do not include cancel jobs as running for scheduled jobs r=pbardea a=pbardea Previously, the query that the job scheduling system would use to detect if there were any already running jobs would include canceled jobs due to using a the alternative spelling: 'cancelled'. This commit changes the query to use the enums instead of manually listing their state. It also adds a test to ensure that jobs are still run, regardless of the wait policy when all previous runs are in a terminal state. Release note: None 51194: pgwire: de-strictify extended protocol handling r=jordanlewis a=jordanlewis Fixes #33693. Fixes #41511. Previously, the protocol handler didn't permit simple queries during the extended protocol mode. I think this was done because the spec vaguely says that extended protocol commands must be ended with a SYNC command: https://www.postgresql.org/docs/9.3/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY However, an examination of the PostgreSQL source code shows that the extended protocol mode bit is only used for error recovery. If an error is encountered during extended protocol mode in Postgres, all commands are skipped until the next Sync. CockroachDB never implemented that behavior - instead, it used the extended protocol mode bit only to reject simple queries if they came before the Sync. This commit removes the extended protocol mode bit, as the use case we used it for was incorrect. It's unclear to me whether we need to re-add the bit for dealing with error cases, but we haven't needed it yet. Adding that might be follow-on work, and won't come in this PR. Release note (bug fix): prevent spurious "SimpleQuery not allowed while in extended protocol mode" errors. 51218: Fix typo in txn.commits metric help text r=nvanbenschoten a=a-robinson Sorry for the mostly useless change, but it bothered me :) 51226: opt: fix error in case of casted NULL arguments to AddGeometryColumn r=rytaft a=rytaft This commit fixes an error that occurred when `AddGeometryColumn` was called with `NULL` arguments that were cast to the type specified by the function signature. #50992 already fixed the case when `AddGeometryColumn` was called with bare `NULL` arguments, since those were detected by `TypeCheck`. `TypeCheck` does not detect `NULL` arguments if they are cast to the correct type. This commit fixes the error by adding an explicit check in the `optbuilder` that each argument is not null before calling the `SQLFn` of the `AddGeometryColumn` overload. Informs #50296 Release note (bug fix): Fixed an internal error that occurred when AddGeometryColumn was called with NULL arguments. This now results in a no-op and returns NULL. Co-authored-by: Radu Berinde <[email protected]> Co-authored-by: anzoteh96 <[email protected]> Co-authored-by: Paul Bardea <[email protected]> Co-authored-by: Jordan Lewis <[email protected]> Co-authored-by: Alex Robinson <[email protected]> Co-authored-by: Rebecca Taft <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This commit fixes an error that occurred when
AddGeometryColumnwascalled with
NULLarguments. The error was caused by an assertion thatthe output of
TypeCheckwas atree.FuncExprwhen in fact it wastree.DNull.This commit fixes the error by adding an explicit check for
tree.DNullafter calling
TypeCheck.Fixes #50296
Release note (bug fix): Fixed an internal error that occurred when
AddGeometryColumn was called with NULL arguments. This now results in
a no-op and returns NULL.