1. 19 Sep, 2020 1 commit
    • craig[bot]'s avatar
      Merge #53831 · d6f91638
      craig[bot] authored
      53831: workload: add interleavebench workload r=yuzefovich a=yuzefovich
      
      This commit adds `interleavebench` workload that can be used to benchmark
      DELETE queries for interleaved and non-interleaved tables. It supports
      different number of levels (minimum 2), custom ratio in number of rows
      between levels, and several other parameters.
      
      As an example, for the following initialization
      ```
      ./bin/workload init interleavebench --drop --interleave=false --levels=3 --hierarchy=1,3,2 --ratio=10 --intra-level-ratio=0.5
      ```
      these queries will be executed:
      ```
      CREATE TABLE table0_0 (c0 INT, PRIMARY KEY (c0));
      CREATE TABLE table1_0 (c0 INT, c1 INT, PRIMARY KEY (c0, c1));
      CREATE TABLE table1_1 (c0 INT, c1 INT, PRIMARY KEY (c0, c1));
      CREATE TABLE table1_2 (c0 INT, c1 INT, PRIMARY KEY (c0, c1));
      CREATE TABLE table2_0 (c0 INT, c1 INT, c2 INT, PRIMARY KEY (c0, c1, c2));
      CREATE TABLE table2_1 (c0 INT, c1 INT, c2 INT, PRIMARY KEY (c0, c1, c2));
      ALTER TABLE table1_0 ADD CONSTRAINT fk FOREIGN KEY (c0) REFERENCES table0_0(c0) ON DELETE CASCADE;
      ALTER TABLE table1_1 ADD CONSTRAINT fk FOREIGN KEY (c0) REFERENCES table0_0(c0) ON DELETE CASCADE;
      ALTER TABLE table1_2 ADD CONSTRAINT fk FOREIGN KEY (c0) REFERENCES table0_0(c0) ON DELETE CASCADE;
      ALTER TABLE table2_0 ADD CONSTRAINT fk FOREIGN KEY (c0, c1) REFERENCES table1_0(c0, c1) ON DELETE CASCADE;
      ALTER TABLE table2_1 ADD CONSTRAINT fk FOREIGN KEY (c0, c1) REFERENCES table1_0(c0, c1) ON DELETE CASCADE;
      INSERT INTO table0_0 (SELECT i FROM generate_series(1, 100) AS i);
      INSERT INTO table1_0 (SELECT floor((i-1)*0.10)::INT+1, i FROM generate_series(1, 1000) AS i);
      INSERT INTO table1_1 (SELECT floor((i-1)*0.20)::INT+1, i FROM generate_series(1, 500) AS i);
      INSERT INTO table1_2 (SELECT floor((i-1)*0.40)::INT+1, i FROM generate_series(1, 250) AS i);
      INSERT INTO table2_0 (SELECT floor((i-1)*0.01)::INT+1, floor((i-1)*0.10)::INT+1, i FROM generate_series(1, 10000) AS i);
      INSERT INTO table2_1 (SELECT floor((i-1)*0.02)::INT+1, floor((i-1)*0.20)::INT+1, i FROM generate_series(1, 5000) AS i);
      ```
      
      Then, during `run` the workload can execute DELETE queries like:
      ```
      DELETE FROM table0 WHERE c0 IN ($1, $2, ...);
      ```
      or
      ```
      DELETE FROM table0 WHERE c0 >= $1 AND c0 < $1 + 1;
      ```
      The table from which to delete as well as the number of rows to be deleted
      by one query can be customized.
      
      The workload does keep track of which rows are deleted in order to issue
      non-overlapping deletes. That is why it ignores `concurrency` flag and
      always runs with no concurrency.
      
      Fixes: #53455.
      
      Release note: None
      Co-authored-by: default avatarYahor Yuzefovich <[email protected]>
      d6f91638
  2. 18 Sep, 2020 24 commits
    • craig[bot]'s avatar
      Merge #54485 · 714ffe25
      craig[bot] authored
      54485: sql: implement SHOW GRANTS ON TYPE r=solongordon a=otan
      
      Resolves: #53569 
      
      **sql: implement SHOW GRANTS ON TYPE**
      
      Release note (sql change): Implement the `SHOW GRANTS ON TYPE` command,
      which shows grants for a specific type.
      
      **sql: add information_schema.type_privileges**
      
      This is a custom table as the PostgreSQL data_type_privileges table
      contains different information and has different semantics. This is
      needed for SHOW GRANTS delegate to work as appropriate.
      
      Release note (sql change): Add a custom
      information_schema.type_privileges table that displays type privileges
      for each supported type.
      Co-authored-by: default avatarOliver Tan <[email protected]>
      714ffe25
    • Yahor Yuzefovich's avatar
      workload: add interleavebench workload · 2093328f
      Yahor Yuzefovich authored
      This commit adds `interleavebench` workload that can be used to benchmark
      DELETE queries for interleaved and non-interleaved tables. It supports
      different number of levels (minimum 2), custom ratio in number of rows
      between levels, and several other parameters.
      
      As an example, for the following initialization
      ```
      ./bin/workload init interleavebench --drop --interleave=false --levels=3 --hierarchy=1,3,2 --ratio=10 --intra-level-ratio=0.5
      ```
      these queries will be executed:
      ```
      CREATE TABLE table0_0 (c0 INT, PRIMARY KEY (c0));
      CREATE TABLE table1_0 (c0 INT, c1 INT, PRIMARY KEY (c0, c1));
      CREATE TABLE table1_1 (c0 INT, c1 INT, PRIMARY KEY (c0, c1));
      CREATE TABLE table1_2 (c0 INT, c1 INT, PRIMARY KEY (c0, c1));
      CREATE TABLE table2_0 (c0 INT, c1 INT, c2 INT, PRIMARY KEY (c0, c1, c2));
      CREATE TABLE table2_1 (c0 INT, c1 INT, c2 INT, PRIMARY KEY (c0, c1, c2));
      ALTER TABLE table1_0 ADD CONSTRAINT fk FOREIGN KEY (c0) REFERENCES table0_0(c0) ON DELETE CASCADE;
      ALTER TABLE table1_1 ADD CONSTRAINT fk FOREIGN KEY (c0) REFERENCES table0_0(c0) ON DELETE CASCADE;
      ALTER TABLE table1_2 ADD CONSTRAINT fk FOREIGN KEY (c0) REFERENCES table0_0(c0) ON DELETE CASCADE;
      ALTER TABLE table2_0 ADD CONSTRAINT fk FOREIGN KEY (c0, c1) REFERENCES table1_0(c0, c1) ON DELETE CASCADE;
      ALTER TABLE table2_1 ADD CONSTRAINT fk FOREIGN KEY (c0, c1) REFERENCES table1_0(c0, c1) ON DELETE CASCADE;
      INSERT INTO table0_0 (SELECT i FROM generate_series(1, 100) AS i);
      INSERT INTO table1_0 (SELECT floor((i-1)*0.10)::INT+1, i FROM generate_series(1, 1000) AS i);
      INSERT INTO table1_1 (SELECT floor((i-1)*0.20)::INT+1, i FROM generate_series(1, 500) AS i);
      INSERT INTO table1_2 (SELECT floor((i-1)*0.40)::INT+1, i FROM generate_series(1, 250) AS i);
      INSERT INTO table2_0 (SELECT floor((i-1)*0.01)::INT+1, floor((i-1)*0.10)::INT+1, i FROM generate_series(1, 10000) AS i);
      INSERT INTO table2_1 (SELECT floor((i-1)*0.02)::INT+1, floor((i-1)*0.20)::INT+1, i FROM generate_series(1, 5000) AS i);
      ```
      
      Then, during `run` the workload can execute DELETE queries like:
      ```
      DELETE FROM table0 WHERE c0 IN ($1, $2, ...);
      ```
      or
      ```
      DELETE FROM table0 WHERE c0 >= $1 AND c0 < $1 + 1;
      ```
      The table from which to delete as well as the number of rows to be deleted
      by one query can be customized.
      
      The workload does keep track of which rows are deleted in order to issue
      non-overlapping deletes. That is why it ignores `concurrency` flag and
      always runs with no concurrency.
      
      Release note: None
      2093328f
    • craig[bot]'s avatar
      Merge #54536 #54541 #54558 #54579 #54582 · 3a5c0b93
      craig[bot] authored
      54536: sql: add unimplemented errors for trigrams r=solongordon a=otan
      
      Resolves #51137 
      
      **sql: add unimplemented errors for index with operator classes**
      
      * Amend lexing to do lookahead for NULLS FIRST / NULLS LAST to avoid
        shift/reduce conflicts.
      * Amend index_params support to be Postgre-compatible. Doing a_exprs
        inside indexes now require surrounding braces.
      * Add operator_class argument to index_params.
      * Add unimplemented error for operator class support.
      
      Release note: None
      
      **builtins: add unimplemented notices around trigram builtins**
      
      Release note: None
      
      
      
      54541: opt: build constraint for containment operator r=rytaft a=mgartner
      
      This commit builds a non-tight, non-null constraint for containment
      operators. This is valid because `NULL` cannot contain any elements.
      
      This fixes a minor issue with the statistics of partial inverted index
      scans. Tests for these scans have been added for in this commit.
      
      Release note: None
      
      54558: opt: deduce not-null constraints for functions with non-nullable args r=rytaft a=rytaft
      
      Release justification: low risk, high benefit changes to existing
      functionality, bug fixes and low-risk updates to new functionality
      
      This commit enhances the constraint builder code so that it can deduce
      not-null constraints for any variable arguments to functions with non-
      nullable args. This is important for improving optimizer support for
      spatial functions, since many of these functions have non-nullable args
      and are used as filter predicates. Deducing not-null constraints for
      the arguments to these functions enables us to calculate more accurate
      statistics, and may unlock other optimizations.
      
      Release note (performance improvement): The optimizer can now deduce that
      certain variable arguments to functions must be non-null. This improves
      cardinality estimation for those variables and unlocks other types of
      optimizations. As a result, the optimizer may choose better query plans
      when a function is used as a filter predicate.
      
      54579: parser: add unimplemented for GROUP BY {GROUPING SETS, ROLLUP, CUBE} r=solongordon a=otan
      
      Refs #51424 
      Refs #46280
      
      Release note: None
      
      54582: parser: add unimplemented error for COPY ... WHERE ... r=solongordon a=otan
      
      Refs #54580 
      Refs #51424 
      
      Release note: None
      Co-authored-by: default avatarOliver Tan <[email protected]>
      Co-authored-by: default avatarMarcus Gartner <[email protected]>
      Co-authored-by: default avatarRebecca Taft <[email protected]>
      3a5c0b93
    • Oliver Tan's avatar
      sql: add unimplemented errors for index with operator classes · 3f4528a6
      Oliver Tan authored
      * Amend lexing to do lookahead for NULLS FIRST / NULLS LAST to avoid
        shift/reduce conflicts.
      * Amend index_params support to be Postgre-compatible. Doing a_exprs
        inside indexes now require surrounding braces.
      * Add operator_class argument to index_params.
      * Add unimplemented error for operator class support.
      
      Release note (sql change): add unimplemented errors for using operator
      classes as parameters for creating an index.
      3f4528a6
    • Oliver Tan's avatar
      builtins: add unimplemented notices around trigram builtins · e8a3aba3
      Oliver Tan authored
      Release note: None
      e8a3aba3
    • Oliver Tan's avatar
      parser: add unimplemented for GROUP BY {GROUPING SETS, ROLLUP, CUBE} · 0c604204
      Oliver Tan authored
      Release note: None
      0c604204
    • Oliver Tan's avatar
      parser: add unimplemented error for COPY ... WHERE ... · 383396ca
      Oliver Tan authored
      Release note: None
      383396ca
    • craig[bot]'s avatar
      Merge #54450 #54517 #54519 #54531 · 84d37a72
      craig[bot] authored
      54450: userfile: add userfile telemetry r=pbardea a=adityamaru
      
      Adds telemetry to userfile upload, delete, list, and counts how many
      times userfile is used as the import source.
      
      Informs: #53431
      
      Release note: None
      
      54517: sql: backfill partial inverted indexes r=rytaft a=mgartner
      
      This commit adds support for backfilling partial inverted indexes.
      After the backfill is complete, the number of entries in the index are
      verified to be correct.
      
      Release note: None
      
      54519: sql: implement CREATE EXTENSION syntax r=solongordon a=otan
      
      Refs #51424, #51137 
      
      Release note (sql change): Add support for the CREATE EXTENSION syntax.
      This no-ops for PostGIS, and gives unimplemented errors for extensions
      we do not yet support but may plan to.
      
      54531: sql: don't allow cross-database sequence owners r=RaduBerinde a=RaduBerinde
      
      #### sql: add more tests for cross-db views
      
      Follow-up to #54475, a few more tests around views:
       - reference to sequence from another database
       - CREATE OR REPLACE
      
      Release note: None
      
      #### sql: don't allow cross-database sequence owners
      
      A sequence can be "owned" by a column. This change disallows setting
      owners from tables in other databases and adds a cluster setting to
      allow them (false by default).
      
      Informs #54126.
      
      Release note (sql change): creating sequences that are OWNED BY
      columns in tables in other databases is now disallowed (and can be
      re-enabled via a cluster setting).
      Co-authored-by: default avatarAditya Maru <[email protected]>
      Co-authored-by: default avatarMarcus Gartner <[email protected]>
      Co-authored-by: default avatarOliver Tan <[email protected]>
      Co-authored-by: default avatarRadu Berinde <[email protected]hlabs.com>
      84d37a72
    • craig[bot]'s avatar
      Merge #54565 #54568 #54573 #54575 · d0f3bfdc
      craig[bot] authored
      54565: parser: parse REINDEX SCHEMA r=arulajmani a=otan
      
      Refs: #51424
      
      Release note: None
      
      54568: sql: add unimplemented errors for jsonpath types and builtins r=arulajmani a=otan
      
      Refs #22513 , #51424
      
      Release note: None
      
      54573: builtins: add unimplemented errors for full text search builtins r=arulajmani a=otan
      
      Refs: #7821, #51424 
      
      Release note: None
      
      54575: parser: add unimplemented errors for CREATE/DROP ACCESS METHOD r=arulajmani a=otan
      
      No issue number for this one (same as aggregate) as there's no way I think we can
      realistically support this. Telemetry still there though.
      
      Refs: #51424
      
      Release note: None
      Co-authored-by: default avatarOliver Tan <[email protected]>
      d0f3bfdc
    • Radu Berinde's avatar
      sql: don't allow cross-database sequence owners · 3f99be83
      Radu Berinde authored
      A sequence can be "owned" by a column. This change disallows setting
      owners from tables in other databases and adds a cluster setting to
      allow them (false by default).
      
      Informs #54126.
      
      Release note (sql change): creating sequences that are OWNED BY
      columns in tables in other databases is now disallowed (and can be
      re-enabled via a cluster setting).
      3f99be83
    • Oliver Tan's avatar
      builtins: add unimplemented errors for full text search builtins · b87de7cf
      Oliver Tan authored
      Release note: None
      b87de7cf
    • Oliver Tan's avatar
      parser: add unimplemented errors for CREATE/DROP ACCESS METHOD · 77c69350
      Oliver Tan authored
      Release note: None
      77c69350
    • craig[bot]'s avatar
      Merge #54538 #54539 · 77751ac7
      craig[bot] authored
      54538: sql: update search_path semantics to match pg now that we support UDS r=otan a=arulajmani
      
      Postgres semantics dictate that the default search_path should be
      `$user, public`, where `$user` expands to the current user's username.
      This wasn't a problem until now as we lacked support for user defined
      schemas, which meant a schema by the name of `$user` wouldn't ever
      exist. This however had changed now and this patch brings us in line
      with the PG semantics.
      
      Fixes #53560
      
      Release note (sql change): The default search path for all sessions is
      now `$user, public` (as opposed to just `public`). This affects our
      name resolution semantics -- now, if a table is present in both the
      public schema and the schema named the current user's username, an
      unqualified object name will be searched/placed in the user's schema.
      This doesn't impact the search semantics of tables in
      pg_catalog/information_schema/temp_schema -- these continued to be
      searched before checking the $user schema and the public schema.
      
      54539: bulkio: Propagate errors when executing schedule. r=miretskiy a=miretskiy
      
      Informs #54484
      
      Use correct error object when checking for retryable errors.
      In addition, add a `FOR UPDATE` clause when picking up
      schedules to execute to reduce contention.
      
      Release Notes: None
      
      Release Justification: Bug fix; Incorrect handling of transaction
      errors resulted in scheduled jobs showing incorrect and confusing
      status message.
      Co-authored-by: default avatararulajmani <[email protected]>
      Co-authored-by: default avatarYevgeniy Miretskiy <[email protected]>
      77751ac7
    • Rebecca Taft's avatar
      opt: deduce not-null constraints for functions with non-nullable args · e280f487
      Rebecca Taft authored
      Release justification: low risk, high benefit changes to existing
      functionality, bug fixes and low-risk updates to new functionality
      
      This commit enhances the constraint builder code so that it can deduce
      not-null constraints for any variable arguments to functions with non-
      nullable args. This is important for improving optimizer support for
      spatial functions, since many of these functions have non-nullable args
      and are used as filter predicates. Deducing not-null constraints for
      the arguments to these functions enables us to calculate more accurate
      statistics, and may unlock other optimizations.
      
      Release note (performance improvement): The optimizer can now deduce that
      certain variable arguments to functions must be non-null. This improves
      cardinality estimation for those variables and unlocks other types of
      optimizations. As a result, the optimizer may choose better query plans
      when a function is used as a filter predicate.
      e280f487
    • craig[bot]'s avatar
      Merge #54313 #54542 · 8485afe0
      craig[bot] authored
      54313: sql: set constraint name in pg error for FK violations r=RaduBerinde a=RaduBerinde
      
      This change adds infrastructure to set the "constraint name" field of
      errors on pgwire and uses it to populate the constraint name for
      foreign key violation errors.
      
      Fixes #36494.
      
      Release note (sql change): foreign key violation errors now fill in
      the "constraint name" error message field.
      
      54542: sql: enable partial inverted indexes in randomized tests r=rytaft a=mgartner
      
      Now that partial inverted indexes can be created, they can be generated
      in randomized tests.
      
      Release note: None
      Co-authored-by: default avatarRadu Berinde <[email protected]>
      Co-authored-by: default avatarMarcus Gartner <[email protected]>
      8485afe0
    • Marcus Gartner's avatar
      opt: build constraint for containment operator · a697042e
      Marcus Gartner authored
      This commit builds a non-tight, non-null constraint for containment
      operators. This is valid because `NULL` cannot contain any elements.
      
      This fixes a minor issue with the statistics of partial inverted index
      scans. Tests for these scans have been added for in this commit.
      
      Release note: None
      a697042e
    • Oliver Tan's avatar
      sql: add unimplemented errors for jsonpath types and builtins · 5a476855
      Oliver Tan authored
      Release note: None
      5a476855
    • Oliver Tan's avatar
      parser: parse REINDEX SCHEMA · 161542a4
      Oliver Tan authored
      Refs: #51424
      
      Release note: None
      161542a4
    • craig[bot]'s avatar
      Merge #45966 · 9787fcdb
      craig[bot] authored
      45966: sql: fix internal executor usage in leaf txn r=knz,andreimatei a=andreimatei
      
      The internal executor was setting a priority on a leaf txn, which is not
      allowed. It was supposed to be a no-op since the priority was the same
      as the existing one, but it crashed nonetheless. This patch makes the
      no-op even more no-op.
      
      Fixes #45924
      
      Release note: None
      
      Release justification: crash fix
      Co-authored-by: default avatarAndrei Matei <[email protected]>
      9787fcdb
    • arulajmani's avatar
      sql: update search_path semantics to match pg now that we support UDS · bd03b170
      arulajmani authored
      Postgres semantics dictate that the default search_path should be
      `$user, public`, where `$user` expands to the current user's username.
      This wasn't a problem until now as we lacked support for user defined
      schemas, which meant a schema by the name of `$user` wouldn't ever
      exist. This however had changed now and this patch brings us in line
      with the PG semantics.
      
      Fixes #53560
      
      Release note (sql change): The default search path for all sessions is
      now `$user, public` (as opposed to just `public`). This affects our
      name resolution semantics -- now, if a table is present in both the
      public schema and the schema named the current user's username, an
      unqualified object name will be searched/placed in the user's schema.
      This doesn't impact the search semantics of tables in
      pg_catalog/information_schema/temp_schema -- these continued to be
      searched before checking the $user schema and the public schema.
      bd03b170
    • Yevgeniy Miretskiy's avatar
      bulkio: Propagate errors when executing schedule. · badf836c
      Yevgeniy Miretskiy authored
      Use correct error object when checking for retryable errors.
      In addition, add a `FOR UPDATE` clause when picking up
      schedules to execute to reduce contention.
      
      Release Notes: None
      
      Release Justification: Bug fix; Incorrect handling of transaction
      errors resulted in scheduled jobs showing incorrect and confusing
      status message.
      badf836c
    • Andrei Matei's avatar
      sql: fix internal executor usage in leaf txn · bd255e5d
      Andrei Matei authored
      Internal executors use a conn executor under the hood, which sets a priority
      and configures stepping on the transaction it is provided with. Both operations
      would previously cause panics when used with a leaf transaction.
      
      This commit reworks setting transaction priority on leaf transactions to avoid
      crashing when the new priority is the same as the old one, and allows stepping
      to be configured on a leaf transaction since doing so is a noop.
      
      Fixes #45924
      
      Release note: None
      
      Release justification: crash fix
      bd255e5d
    • craig[bot]'s avatar
      Merge #54283 · 176c08c8
      craig[bot] authored
      54283: execbuilder,invertedexpr,rowexec,...: add pre-filtering to invertedFi… r=sumeerbhola a=sumeerbhola
      
      …lterer
      
      The optional pre-filtering is passed as an expression in the spec,
      and the PreFilterer instantiated in the invertedFilterer using
      the invertedidx.NewBoundPreFilterer() factory method.
      
      Pre-filtering is restricted to geometry and geography expressions
      that are a single function. But using the expression in the spec
      allows for this to be extended without a spec change.
      
      Release note: None
      
      Release justification: The geospatial functionality is
      new for this release so this falls under category 2
      (low-risk updates to new functionality).
      Co-authored-by: default avatarsumeerbhola <su[email protected]>
      176c08c8
    • craig[bot]'s avatar
      Merge #54518 · 9b706fc2
      craig[bot] authored
      54518: colflow: add bytes sent stat on streams r=cathymw a=cathymw
      
      Fixes: #48112
      This commit (follow-up to #47347) adds the "bytes sent" information collected at inboxes to stream statistics.
      Also changed the inline comments from `ioReadingOp` to `ioReader` to be more consistent with the argument name.
      
      The following screenshot shows that the stream stats now include "bytes sent":
      ![image](https://user-images.githubusercontent.com/23270879/93507496-cab3b780-f8eb-11ea-9c25-6f33d65b97c0.png)
      
      Release note (sql change):: EXPLAIN ANALYZE diagrams now contain "bytes
      sent" information on streams.
      Co-authored-by: default avatarCathy <[email protected]>
      9b706fc2
  3. 17 Sep, 2020 15 commits
    • craig[bot]'s avatar
      Merge #54486 · c37cff83
      craig[bot] authored
      54486: sql: deflake TestTemporarySchemaDropDatabase r=arulajmani a=otan
      
      The job looks like it can race with the check, so hide this behind a
      SucceedsSoon.
      
      Resolves #54467.
      
      Release note: None
      Co-authored-by: default avatarOliver Tan <[email protected]>
      c37cff83
    • Marcus Gartner's avatar
      sql: enable partial inverted indexes in randomized tests · c47d2934
      Marcus Gartner authored
      Now that partial inverted indexes can be created, they can be generated
      in randomized tests.
      
      Release note: None
      c47d2934
    • Marcus Gartner's avatar
      sql: backfill partial inverted indexes · e4b95186
      Marcus Gartner authored
      This commit adds support for backfilling partial inverted indexes.
      After the backfill is complete, the number of entries in the index are
      verified to be correct.
      
      Release note: None
      e4b95186
    • sumeerbhola's avatar
      execbuilder,invertedexpr,rowexec,...: add pre-filtering to invertedFilterer · 06910e63
      sumeerbhola authored
      The optional pre-filtering is passed as an expression in the spec,
      and the PreFilterer instantiated in the invertedFilterer using
      the invertedidx.NewBoundPreFilterer() factory method.
      
      Pre-filtering is restricted to geometry and geography expressions
      that are a single function. But using the expression in the spec
      allows for this to be extended without a spec change.
      
      Release note: None
      
      Release justification: The geospatial functionality is
      new for this release so this falls under category 2
      (low-risk updates to new functionality).
      06910e63
    • craig[bot]'s avatar
      Merge #54468 · cb3ecbcd
      craig[bot] authored
      54468: kvclient: fix a request routing bug r=andreimatei a=andreimatei
      
      This patch fixes a bug in the DistSender's interaction with the range
      cache. The DistSender interacts with the cache through an EvictionToken;
      this token is used to update the lease information for a cache entry and
      to evict the entry completely if the descriptor is found to be too stale
      to be useful. The problem was that the eviction does not work as intended
      if a copy of the token had been used to perform a lease update prior.
      Operations that update the cache take a token and return an updated
      token, and only the most up to date such token can be used in order to
      perform subsequent cache updates (in particular, to evict the cache
      entry). The bug was that the DistSender sometimes lost track of this
      most up to date token, and tried to evict using an older copy - which
      eviction was a no-op.
      
      Specifically, the bad scenario was the following:
      - DistSender.sendPartialBatch made a copy of the token and handed it to
        sendToReplicas.
      - sendToReplicas updated the cached lease through this token (through
        tok.UpdateLease). The updated token was further used by sendToReplicas
        but not returned to sendPartialBatch.
      - No replica can serve the request cause the descriptor is stale, and
        control flow gets back to sendPartialBatch.
      - sendPartialBatch uses its own, stale, copy of the token to attempt a
        cache eviction. The cache ignores the eviction request, claiming that
        it has a more up to date entry than the one that sendPartialBatch is
        trying to evict (which is true).
      - sendPartialBatch then asks the cache for a new descriptor, and the
        cache returns the same one as before. It also returns the lease that
        we have added before, but that doesn't help very much.
      - All this can happen over and over again in some case that I don't
        fully understand. In the restore2TB test, what happens is that the
        node that's supposed to be the leaseholder (say, n1), perpetually
        doesn't know about the range, and another node perpetually thinks that
        n1 is the leaseholder. I'm not sure why the latter happens, but this
        range is probably in an unusual state because the request that's
        spinning in this way is an ImportRequest.
      
      This patch fixes the problem by changing how descriptor evictions work:
      instead of comparing a token's pointer to the current cache entry and
      refusing to do anything if they don't match, now evicting a descriptor
      compares the descriptor's value with what's in the cache.
      The cache code generally moves away from comparing pointers anywhere,
      and an EvictionToken stops storing pointers to the cache's insides for
      clarity.
      
      Fixes #54118
      Fixes #53197
      
      Release note: None
      Co-authored-by: default avatarAndrei Matei <[email protected]>
      cb3ecbcd
    • Andrei Matei's avatar
      kvclient: fix a request routing bug · 84fa010a
      Andrei Matei authored
      This patch fixes a bug in the DistSender's interaction with the range
      cache. The DistSender interacts with the cache through an EvictionToken;
      this token is used to update the lease information for a cache entry and
      to evict the entry completely if the descriptor is found to be too stale
      to be useful. The problem was that the eviction does not work as intended
      if a copy of the token had been used to perform a lease update prior.
      Operations that update the cache take a token and return an updated
      token, and only the most up to date such token can be used in order to
      perform subsequent cache updates (in particular, to evict the cache
      entry). The bug was that the DistSender sometimes lost track of this
      most up to date token, and tried to evict using an older copy - which
      eviction was a no-op.
      
      Specifically, the bad scenario was the following:
      - DistSender.sendPartialBatch made a copy of the token and handed it to
        sendToReplicas.
      - sendToReplicas updated the cached lease through this token (through
        tok.UpdateLease). The updated token was further used by sendToReplicas
        but not returned to sendPartialBatch.
      - No replica can serve the request cause the descriptor is stale, and
        control flow gets back to sendPartialBatch.
      - sendPartialBatch uses its own, stale, copy of the token to attempt a
        cache eviction. The cache ignores the eviction request, claiming that
        it has a more up to date entry than the one that sendPartialBatch is
        trying to evict (which is true).
      - sendPartialBatch then asks the cache for a new descriptor, and the
        cache returns the same one as before. It also returns the lease that
        we have added before, but that doesn't help very much.
      - All this can happen over and over again in some case that I don't
        fully understand. In the restore2TB test, what happens is that the
        node that's supposed to be the leaseholder (say, n1), perpetually
        doesn't know about the range, and another node perpetually thinks that
        n1 is the leaseholder. I'm not sure why the latter happens, but this
        range is probably in an unusual state because the request that's
        spinning in this way is an ImportRequest.
      
      This patch fixes the problem by changing how descriptor evictions work:
      instead of comparing a token's pointer to the current cache entry and
      refusing to do anything if they don't match, now evicting a descriptor
      compares the descriptor's value with what's in the cache.
      The cache code generally moves away from comparing pointers anywhere,
      and an EvictionToken stops storing pointers to the cache's insides for
      clarity.
      
      Fixes #54118
      Fixes #53197
      
      Release note: None
      84fa010a
    • craig[bot]'s avatar
      Merge #54523 · 6b11693d
      craig[bot] authored
      54523: invertedidx: don't do *types.T pointer comparison r=sumeerbhola a=sumeerbhola
      
      Release note: None
      Release justification: Bug fixes and low-risk updates to new functionality
      Co-authored-by: default avatarsumeerbhola <[email protected]>
      6b11693d
    • Radu Berinde's avatar
      sql: add more tests for cross-db views · 21153e22
      Radu Berinde authored
      Follow-up to #54475, a few more tests around views:
       - reference to sequence from another database
       - CREATE OR REPLACE
      
      Release note: None
      21153e22
    • Cathy's avatar
      colflow: add bytes read stat on streams · acbc0116
      Cathy authored
      This commit adds the "bytes sent" information collected at
      inboxes to stream statistics. Also changed the inline comments
      from ioReadingOp to ioReader to be more consistent with the argument name.
      
      Release note (sql change): EXPLAIN ANALYZE diagrams now contain "bytes
      sent" information on streams.
      acbc0116
    • craig[bot]'s avatar
      Merge #54381 #54482 #54497 · 12f55b2c
      craig[bot] authored
      54381: changefeedccl: add metrics for number of job failures and running feeds r=ajwerner a=ajwerner
      
      This commit adds two new metrics:
      1) changefeeds.running - gauge of the number of running CHANGEFEEDs both
           as jobs and sinkless
          
       2) changefeeds.failures - counter of the number of permanent failures of
          CHANGEFEED jobs.
      
      Fixes #53130.
          
      Release note (enterprise change): Added metrics to track the current number
      of running CHANGEFEEDs and the number of failed CHANGEFEED jobs.
      
      
      54482: sql: add parens around partial index in pg_catalog r=mgartner a=rafiss
      
      This improves compatibility with tools that inspect pg_catalog to get
      index definitions. Some, like activerecord, rely on regexps that expect
      these parens.
      
      Release note (sql change): Partial index definitions in pg_catalog are
      now formatted with parentheses around the WHERE clause.
      
      54497: jobs: ensure that the StartedMicros field is populated r=dt a=ajwerner
      
      This commit ensures that when jobs of the new sqlliveness leases are started
      for they first time, they populate their StartedMicros field.
      
      Release note (bug fix): Fixed bug from earlier alphas whereby jobs would not
      properly populate their `started` timestamp.
      Co-authored-by: default avatarAndrew Werner <[email protected]>
      Co-authored-by: default avatarRafi Shamim <[email protected]>
      12f55b2c
    • craig[bot]'s avatar
      Merge #54463 · d0db9ffc
      craig[bot] authored
      54463: sql: allow parsing of REASSIGN OWNED BY ... TO ... r=angelapwen a=angelapwen
      
      Part 1 of addressing #52022.
      
      Add parsing for REASSIGN OWNED BY ... TO ... command. Plan to follow up with implementation of the command for feature parity with PostgresQL, which will resolve the issue linked.
      
      Release note: None
      Co-authored-by: default avatarangelapwen <[email protected]>
      d0db9ffc
    • Oliver Tan's avatar
      sql: implement SHOW GRANTS ON TYPE · 040d6c36
      Oliver Tan authored
      Release note (sql change): Implement the `SHOW GRANTS ON TYPE` command,
      which shows grants for a specific type.
      
      Release note (sql change): The `SHOW GRANTS` without any
      table/type/schema/database qualifier now also shows types. The
      `table_name` column is renamed to `type_name`.
      040d6c36
    • sumeerbhola's avatar
      invertedidx: don't do *types.T pointer comparison · ec27ce53
      sumeerbhola authored
      Release note: None
      Release justification: Bug fixes and low-risk updates to new functionality
      ec27ce53
    • craig[bot]'s avatar
      Merge #54364 · 2d6a2809
      craig[bot] authored
      54364: geoindex: use circumference of earth as default parameters r=sumeerbhola a=otan
      
      Use bounds for the index as a number similar to the circumference of the
      Earth. This is roughly in line for the same value used for SRID 3857,
      which represents the entire planet.
      
      Release note: None
      Co-authored-by: default avatarOliver Tan <[email protected]>
      2d6a2809
    • Oliver Tan's avatar
      sql: implement CREATE EXTENSION syntax · c2829324
      Oliver Tan authored
      Release note (sql change): Add support for the CREATE EXTENSION syntax.
      This no-ops for PostGIS, and gives unimplemented errors for extensions
      we do not yet support but may plan to.
      c2829324