opt: AddGeometryColumn with NULL arguments internal errors #50296
Assignees
Comments
|
Hi @otan, please add a C-ategory label to your issue. Check out the label system docs. While you're here, please consider adding an A- label to help keep our repository tidy. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
otan
added
A-spatial
craig bot
pushed a commit
that referenced
this issue
Jul 6, 2020
50790: geodist: use Point struct instead of a Point interface r=sumeerbhola a=otan Getting an Edge is expensive as the generating a Point that complies with the geodist.Point interface involves mallocing from memory. We call Edge(x) a lot when doing POLYGON <=> POLYGON comparisons. Instead, change the Point to a struct, which does not require any malloc to generate interfaces at the cost of code ugliness. Without bounding box checks, this speeds up a query from 2min to 1min30sec on a ~120000 row join. Release note: None 50962: util/mon: don't crash when shrinking by too much r=yuzefovich a=yuzefovich Previously, we called `panic` when `Shrink`ing the memory account by too much. Such scenario indicates that our memory accounting is off, but we shouldn't be crashing - we should report an error instead, and this commit does that. Fixes: #50804. Release note (bug fix): Previously, CockroachDB could crash when internal memory accounting hit a discrepancy. Now it will report an error instead. 50992: opt: fix error in case of NULL arguments to AddGeometryColumn r=yuzefovich a=rytaft 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 #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: Oliver Tan <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: Rebecca Taft <[email protected]>
rytaft
added a commit
to rytaft/cockroach
that referenced
this issue
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 issue
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
Describe the problem
When using NULL for AddGeometryColumn arguments, an error occurs.
To Reproduce
Expected behavior
This should no-op and return NULL (this seems to be the behaviour of postgres)
The text was updated successfully, but these errors were encountered: