geo: support storage of Z and M dimensions #53091

otan · 2020-08-19T23:09:18Z

We currently don't support storage of Z and M dimensions.

Blocks:

This is largely because our WKT parser does not support Z and M dimensionality. Ideally, make go-geom work (WKT decoder could use re2c twpayne/go-geom#180)
Resolve why geos: do not set 3D on EWKB writer #53909 fails on certain linux systems (e.g. roachprod)

Prototype available here: #53089

blathers-crl · 2020-08-19T23:09:19Z

Hi @otan, I've guessed the C-ategory of your issue and suitably labeled it. Please re-label if inaccurate.

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.}

59690: geo/wkt: implement parser for points with Z and M dimensions r=otan a=andyyang890 This patch adds a parser that is capable of parsing WKT representations of points with Z and M dimensions. Refs: #53091 Release note: None Co-authored-by: Andy Yang <[email protected]>

59740: geo/wkt: add support for parsing linestrings with Z and M dimensions r=otan a=andyyang890 This patch extends the capabilities of the WKT parser to include parsing of linestrings with Z and M dimensions. This patch also improves the error messages generated for parsing errors. Refs: #53091 Release note: None Co-authored-by: Andy Yang <[email protected]>

59830: geo/wkt: add support for parsing polygons with Z and M dimensions r=otan a=andyyang890 This patch extends the capabilities of the WKT parser to include parsing of polygons with Z and M dimensions. Refs: #53091 Release note: None Co-authored-by: Andy Yang <[email protected]>

59745: go.mod: update goavro from v2.8.1 to v2.9.8 r=pbardea a=pbardea Release note: None 60013: geo/wkt: add support for parsing multipoints with Z and M dimensions r=otan a=andyyang890 This patch extends the capabilities of the WKT parser to include parsing of multipoints with Z and M dimensions. Refs: #53091 Release note: None Co-authored-by: Paul Bardea <[email protected]> Co-authored-by: Andy Yang <[email protected]>

59330: sql: make internal executor streaming r=yuzefovich a=yuzefovich This commit updates the internal executor to operate in a streaming fashion by refactoring its internal logic to implement an iterator pattern. A new method `QueryInternalEx` (and its counterpart `QueryInternal`) is introduced (both not used currently) while all existing methods of `InternalExecutor` interface are implemented using the new iterator logic. The communication between the iterator goroutine (the receiver) and the connExecutor goroutine (the sender) is done via a buffered (of 32 size in non-test setting) channel. The channel is closed when the connExecutor goroutine exits its run() loop. Care needs to be taken when closing the iterator - we need to make sure to close the stmtBuf (so that there are no more commands for the connExecutor goroutine to execute) and then we need to unblockingly drain the channel (since the connExecutor goroutine might be blocked on adding a row to the channel). After that we have to wait for the connExecutor goroutine to exit so that we can finish the tracing span. For convenience purposes, if the iterator is fully exhausted, it will get closed automatically. Addresses: #48595. Release note: None 60023: kv: wait for replica initialization in closed timestamp tests r=nvanbenschoten a=nvanbenschoten Fixes #60288. This commit updates tests in closed_timestamp_test.go to wait for replica initialization before returning replicas in `replsForRange`. This was causing flakiness on master. I bisected it back to c44b357. It appears that that change allowed for calls directly to `Replica.Send` (testing only) to make it deeper into replica code and eventually hit a panic in `Replica.checkSpanInRangeRLocked`. This commit fixes this in two ways: 1. it updates the closed timestamp tests to wait for replica initialization before calling directly into `Replica.Send`. Tests shouldn't be calling `Replica.Send` on uninitialized replicas. 2. it adds extra protection in `Replica.checkExecutionCanProceed` to throw an error if the replica is no initialized. This isn't strictly necessary, but it's cheap and limits the blast radius of getting this wrong elsewhere. Release note: None 60282: delegate: gate estimated_row_count on SHOW TABLES behind cluster setting r=rafiss a=otan Refs: #58189 Release note (sql change): Introduced a cluster setting sql.show_tables.estimated_row_count.enabled, which defaults to true. If false, estimated_row_count will not display on SHOW TABLES which improves performance. 60285: geo/wkt: add support for parsing multilinestrings with Z and M dims r=otan a=andyyang890 This patch extends the capabilities of the WKT parser to include parsing of multilinestrings with Z and M dimensions. Refs: #53091 Release note: None 60305: sql: use kv_node_status to determine node liveness for multiregion r=arulajmani a=otan This was a relic before the decision was made -- node liveness was detected by gossip instead. This changes the code to use SHOW REGIONS FROM CLUSTER, which aggregates from kv_node_status. Release note: None Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Oliver Tan <[email protected]> Co-authored-by: Andy Yang <[email protected]>

60427: ui: upgrade deps with security vulns r=dhartunian a=dhartunian - upgrade version of lodash - upgrade resolution version of ua-parser - upgrade version of analytics-node - upgrade version of highlight.js and type definitions Release note: None 60449: krb5: fix configure script r=RaduBerinde a=otan This should read "test", not "text". Release note: None 60450: geo/wkt: add support for parsing multipolygons with Z and M dimensions r=otan a=andyyang890 This patch extends the capabilities of the WKT parser to include parsing of multipolygons with Z and M dimensions. Refs: #53091 Release note: None Co-authored-by: David Hartunian <[email protected]> Co-authored-by: Oliver Tan <[email protected]> Co-authored-by: Andy Yang <[email protected]>

59865: sql: add schema_name,table_id to crdb_internal.ranges r=rafiss a=jordanlewis ... and crdb_internal.ranges_no_leases Closes #59601. This commit adds schema_name to crdb_internal.ranges and crdb_internal.ranges_no_leases to ensure that it's possible to disambiguate between ranges that are contained by a table with the same name in two different user-defined schemas. In addition, it also adds the table_id column which allows unambiguous lookups of ranges for a given table id. This will also enable making a virtual index on the table_id column later, which should be a nice win for some introspection commands. Release note (sql change): add the schema_name and table_id columns to the crdb_internal.ranges and crdb_internal.ranges_no_leases virtual tables. 60546: kvserver: improve handling for removal of a replica, when multiple replicas already exist on the same node r=lunevalex a=lunevalex Fixes #60545 The allocator in some cases allows for a range to have a replica on multiple stores of the same node. If that happens, it should allow itself to fix the situation by removing one of the offending replicas. This was only half working due to an ordering problem in how the replicas appeared in the descriptor. It could remove the first replica, but not the second one. . Release note: None 60561: geo/wkt: simplify parser grammar and improve error messages r=otan a=andyyang890 This patch simplifies the yacc grammar for the WKT parser and also improves the error messages for mixed dimensionality problems. Refs: #53091 Release note: None Co-authored-by: Jordan Lewis <[email protected]> Co-authored-by: Alex Lunev <[email protected]> Co-authored-by: Andy Yang <[email protected]>

60515: geo/wkt: add support for parsing geometrycollections with Z and M dims r=otan a=andyyang890 This patch extends the capabilities of the WKT parser to include parsing of geometrycollections with Z and M dimensions. Refs: #53091 Release note: None 60762: sql: ensure graceful rollbacks if the ADD REGION async job fails r=otan a=arulajmani Previously, we assumed that the async component of an `ALTER DATABASE ... ADD REGION` wouldn't fail. This isn't really true though, because users may cancel jobs or we may be dropping regions in the same txn that fails validation. We have validation that ensures every region (READ ONLY or PUBLIC) on the multi-region type descriptor is also present on the database descriptor's region config (and vice-versa). If we realize that a region can't be promoted from READ ONLY to PUBLIC, we must also remove it from the database descriptor's region config (in addition removing it from the type descriptor). Prior to this patch we didn't do so, and cleanup could potentially fail validation. This patch fixes that behavior. This is a temporary band-aid until we stop denormalizing region values and have a single source of truth. Release note: None Co-authored-by: Andy Yang <[email protected]> Co-authored-by: arulajmani <[email protected]>

59566: kvserver: introduce a Raft-based transport for closedts r=andreimatei a=andreimatei This patch introduces a replacement for the existing closed timestamp mechanism / transport. The new mechanism is gated by a cluster version. Raft commands now carry increasing closed timestamps generated by the propBuf by using the recent request Tracker for synchronizing with in-flight requests (i.e. not closing timestamps below them). Raft commands get a closed ts field, and the range state gets the field as well. The propBuf pays attention to the range's closed timestamp policy for deciding whether to close lagging or leading timestamps. Release note: None 60753: kv: add TestStoreRangeSplitAndMergeWithGlobalReads r=nvanbenschoten a=nvanbenschoten Made possible by #60567. This commit adds a new test called TestStoreRangeSplitAndMergeWithGlobalReads that tests that a range configured to serve global reads can be split and merged. In essence, this tests whether the split and merge transactions can handle having their timestamp bumped by the closed timestamp on the ranges they're operating on. The test revealed that range merges did have issues in these cases, because SubsumeRequests assumed that the intent on the RHS's local descriptor was below the node's HLC clock. This is no longer always true, so we now perform the inconsistent scan at hlc.MaxTimestamp, just like QueryIntent requests do. 60772: sql: add parse_timestamp builtin r=RaduBerinde a=RaduBerinde Add a builtin that can be used to parse timestamp strings. This is like a cast, but it does not accept relative timestamps so it can be immutable. Only immutable expressions are allowed in computed column expressions or partial index predicates; unlike casts, the new function can be used in such expressions. Fixes #60578. Release notes (sql change): A new parse_timestamp function can be used to parse absolute timestamp strings in computed column expressions or partial index predicates. 60796: geo: replace GEOS with new WKT parser for EWKT parsing r=otan a=andyyang890 Previously, the `parseEWKT` function used GEOS to parse WKT strings. This was inadequate because GEOS has issues with parsing Z and M dimensions. To address this, a new WKT parser was implemented with goyacc and this patch integrates it into CockroachDB. Refs: #53091 Release note: None 60818: opt: add partition info to the opt catalog r=rytaft a=rytaft Prior to this commit, the opt catalog did not include zone information or prefixes specific to each partition of an index. This commit adds this information since it will be necessary to support locality optimized search in a future commit. Informs #55185 Release note: None Co-authored-by: Andrei Matei <[email protected]> Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Radu Berinde <[email protected]> Co-authored-by: Andy Yang <[email protected]> Co-authored-by: Rebecca Taft <[email protected]>

60964: kvserver: fix TestBadRequest r=lunevalex a=lunevalex Fixes #51795 This test was originally written when the test infra started up with a single range. This commit updates the logic to work with any config. To make the last error fire, we need to contain the delete span to a single range that starts at KeyMin. Release note: None 60971: geo/wkt: add line and pos numbers to WKT parser syntax errors r=otan a=andyyang890 This patch enhances the WKT parser to show line and position numbers in syntax error messages. Refs: #53091 Release note: None Co-authored-by: Alex Lunev <[email protected]> Co-authored-by: Andy Yang <[email protected]>

61113: ui: show replica type on the range report page r=aayushshah15 a=aayushshah15 Resolves #59677 Release justification: observability improvement Release note (ui change): the range report page on the admin ui will now also show each of the replica's types 61128: jobs: introduce jobspb.JobID r=lucy-zhang a=lucy-zhang This commit introduces a `jobspb.JobID` int64 type and uses it in most places where we were previously using an int64. Closes #61121. Release justification: Low-risk change to existing functionality. Release note: None 61129: geo/wkt: update parsing of dimensions for empty geometrycollections r=otan,rafiss a=andyyang890 Previously, the data structure used for storing geometry collections was unable to store a layout, which made it impossible to distinguish empty geometry collections of different layouts. That issue has since been fixed and this patch updates the parser accordingly. Resolves #61035. Refs: #53091 Release justification: bug fix for new functionality Release note: None 61130: kv: disable timestamp cache + current clock assertion r=nvanbenschoten a=nvanbenschoten Closes #60580. Closes #60736. Closes #60779. Closes #61060. This was added in 218a5a3. The check was more of a sanity check that we have and always have had an understand of the timestamps that can enter the timestamp cache. The fact that it's failing is a clear indication that there were issues in past releases, because a lease transfer used to only be safe if the outgoing leaseholder's clock was above the time of any read in its timestamp cache. We now ship a snapshot of the timestamp cache on lease transfers, so that invariant is less important. I'd still like to get to the bottom of this, but I'll do so on my own branch, off of master where it's causing disruption. Release justification: avoid assertion failures 61155: jobs: make sure we finish spans if canceled before starting job r=ajwerner a=ajwerner Was seeing: ``` testcluster.go:135: condition failed to evaluate within 45s: unexpectedly found active spans: 0.000ms 0.000ms === operation:job _unfinished:1 intExec:create-stats goroutine 84 [running]: runtime/debug.Stack(0xc0086b1890, 0x792e940, 0xc009ac37e0) /usr/local/go/src/runtime/debug/stack.go:24 +0xab ``` In roachprod stressrace with a big cluster. This seemed to fix it. Release justification: bug fixes and low-risk updates to new functionality. Release note: None Co-authored-by: Aayush Shah <[email protected]> Co-authored-by: Lucy Zhang <[email protected]> Co-authored-by: Andy Yang <[email protected]> Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Andrew Werner <[email protected]>

blathers-crl bot added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Aug 19, 2020

otan mentioned this issue Sep 3, 2020

geos: do not set 3D on EWKB writer #53909

Closed

otan mentioned this issue Nov 2, 2020

20.2 known limitations cockroachdb/docs#8784

Merged

andyyang890 self-assigned this Jan 21, 2021

andyyang890 mentioned this issue Feb 2, 2021

geo/wkt: implement parser for points with Z and M dimensions #59690

Merged

andyyang890 mentioned this issue Feb 4, 2021

geo/wkt: add support for parsing linestrings with Z and M dimensions #59740

Merged

andyyang890 mentioned this issue Feb 5, 2021

geo/wkt: add support for parsing polygons with Z and M dimensions #59830

Merged

andyyang890 mentioned this issue Feb 8, 2021

geo/wkt: add support for parsing multipoints with Z and M dimensions #60013

Merged

andyyang890 mentioned this issue Feb 9, 2021

geo/wkt: add support for parsing multilinestrings with Z and M dims #60285

Merged

andyyang890 mentioned this issue Feb 10, 2021

geo/wkt: add support for parsing multipolygons with Z and M dimensions #60450

Merged

This was referenced Feb 12, 2021

geo/wkt: add support for parsing geometrycollections with Z and M dims #60515

Merged

geo/wkt: simplify parser grammar and improve error messages #60561

Merged

andyyang890 mentioned this issue Feb 19, 2021

geo: replace GEOS with new WKT parser for EWKT parsing #60796

Merged

andyyang890 mentioned this issue Feb 20, 2021

geo: add support for storing spatial types with Z and M dimensions #60832

Merged

craig bot closed this as completed in 682582f Feb 21, 2021

andyyang890 mentioned this issue Feb 23, 2021

geo/wkt: add line and pos numbers to WKT parser syntax errors #60971

Merged

andyyang890 mentioned this issue Feb 25, 2021

geo/wkt: update parsing of dimensions for empty geometrycollections #61129

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

geo: support storage of Z and M dimensions #53091

geo: support storage of Z and M dimensions #53091

otan commented Aug 19, 2020 •

edited

Loading

blathers-crl bot commented Aug 19, 2020

geo: support storage of Z and M dimensions #53091

geo: support storage of Z and M dimensions #53091

Comments

otan commented Aug 19, 2020 • edited Loading

blathers-crl bot commented Aug 19, 2020

otan commented Aug 19, 2020 •

edited

Loading