This project is mirrored from https://github.com/cockroachdb/cockroach. Pull mirroring updated .
  1. 28 Feb, 2021 4 commits
    • craig[bot]'s avatar
      Merge #61232 · 4bb49c39
      craig[bot] authored
      61232: cli: ensure that `--log` is recognized for non-server commands r=aaron-crl a=knz
      
      cc @taroface 
      
      Release justification: bug fixes and low-risk updates to new functionality
      
      Release note (bug fix): The non-server `cockroach` commands now
      recognize the new `--log` flag properly. This had been broken
      unadvertently in one of the earlier 21.1 alpha releases.
      Co-authored-by: default avatarRaphael 'kena' Poss <[email protected]>
      4bb49c39
    • Raphael 'kena' Poss's avatar
      cli: ensure that `--log` is recognized for non-server commands · d1cd456a
      Raphael 'kena' Poss authored
      Release justification: bug fixes and low-risk updates to new functionality
      
      Release note (bug fix): The non-server `cockroach` commands now
      recognize the new `--log` flag properly. This had been broken
      unadvertently in one of the earlier 21.1 alpha releases.
      d1cd456a
    • craig[bot]'s avatar
      Merge #61101 · 90284336
      craig[bot] authored
      61101: server: bug fix to correct path checks for existing certificates prior to generation r=aaron-crl a=aaron-crl
      
      I erred in my use of os.Stat and no exists checks were correct. I've refactored the read/write
      functions to perform these checks themselves. This also improves our resiliency against
      races between time of check and time of use for cert/key file paths.
      
      Part of #60632 
      
      Release justification: bug fix to enable proper automatic generation of certificates
      Release note: None
      Co-authored-by: default avatarAaron Blum <[email protected]>
      90284336
    • craig[bot]'s avatar
      Merge #58124 · 7d1324fa
      craig[bot] authored
      58124: server: introduce a package for probing the KV layer r=knz a=joshimhoff
      
      https://docs.google.com/document/d/1NqsIgizseyMxUBimpE10m6sSnbebQ25VyJRUwxdmJyM/edit
      
      **kv: introduce a package for probing the KV layer**
          
      This commmit introduces a package for probing the KV layer. The goal
      of sending such probes is greater obserability into the reliability of
      the KV layer & below. In particular probes may make for a good SLI
      metric. The control we have over the prober workload will generate data
      with a high signal to noise ratio. This lets us alert operators with
      confidence.
      
      How does the prober work? Right now, it just does reads. In a
      loop, it randomly selects a range to probe by scanning meta2.
      It then does a single row read of the start key of that range via
      *kv.DB.
      
      Note that the prober is disabled by default. This commit introduces a
      private cluster setting that defaults to off.
      
      Release justification: auxiliary system that is off by default
      
      Release note: None.
      Co-authored-by: default avatarJosh Imhoff <[email protected]>
      7d1324fa
  2. 27 Feb, 2021 9 commits
    • Josh Imhoff's avatar
      kv: introduce a package for probing the KV layer · 531cdaeb
      Josh Imhoff authored
      Release justification: Auxiliary system that is off by default.
      
      This commmit introduces a package for probing the KV layer. The goal
      of sending such probes is greater obserability into the reliability of
      the KV layer & below. In particular probes may make for a good SLI
      metric. The control we have over the prober workload will generate data
      with a high signal to noise ratio. This lets us alert operators with
      confidence.
      
      How does the prober work? Right now, it just does reads. In a
      loop, it randomly selects a range to probe by scanning meta2.
      It then does a single row read of the start key of that range via
      *kv.DB.
      
      Note that the prober is disabled by default. This commit introduces a
      private cluster setting that defaults to off.
      
      Release note: None.
      531cdaeb
    • craig[bot]'s avatar
      Merge #61208 · 4119ed03
      craig[bot] authored
      61208: kv: disable race testing of kvserver package r=RaduBerinde a=RaduBerinde
      
      The thread sanitizer dies with a "clock allocator overflow" on this
      package. The failure is not specific to a test; we have skipped the
      tests that were running and it's now being hit during another test.
      
      I tried splitting out the client tests, but they rely on a lot of
      _test code from the `kvserver` package. It requires duplicating or
      moving around a lot of code (and making many methods public from
      non-test kvserver code).
      
      Until we figure out a solution, I am skipping the entire package under
      race.
      
      Informs #61120.
      
      Release justification: non-production code change.
      
      Release note: None
      Co-authored-by: default avatarRadu Berinde <[email protected]>
      4119ed03
    • craig[bot]'s avatar
      Merge #61110 #61170 · dcc8ab04
      craig[bot] authored
      61110: kv: implement commit-wait for future time transaction commits r=nvanbenschoten a=nvanbenschoten
      
      Fixes #57687.
      Related to #52745.
      
      This PR introduces a "commit-wait" sleep stage after a transaction commits, which is entered if doing so is deemed necessary for consistency.
      
      By default, commit-wait is only necessary for transactions that commit with a future-time timestamp that leads the local HLC clock. This is because CockroachDB's consistency model depends on all transactions waiting until their commit timestamp is below their gateway clock. In doing so, transactions ensure that at the time that they complete, all other clocks in the system (i.e. on all possible gateways) will be no more than the max_offset below the transaction's commit timestamp. This property ensures that all causally dependent transactions will have an uncertainty interval (see GlobalUncertaintyLimit) that exceeds the original transaction's commit timestamp, preventing stale reads. Without the wait, it would be possible for a read-write transaction to write a future-time value and then for a causally dependent transaction to read below that future-time value, violating "read your writes".
      
      The property must also hold for read-only transactions, which may have a commit timestamp in the future due to an uncertainty restart after observing a future-time value in their uncertainty interval. In such cases, the property that the transaction must wait for the local HLC clock to exceed its commit timestamp is not necessary to prevent stale reads, but it is necessary to ensure monotonic reads. Without the wait, it would be possible for a read-only transaction coordinated on a gateway with a fast clock to return a future-time value and then for a causally dependent read-only transaction coordinated on a gateway with a slow clock to read below that future-time value, violating "monotonic reads".
      
      In practice, most transactions do not need to wait at all, because their commit timestamps were pulled from an HLC clock (i.e. are not synthetic) and so they will be guaranteed to lead the local HLC's clock, assuming proper HLC time propagation. Only transactions whose commit timestamps were pushed into the future will need to wait, like those who wrote to a global_read range and got bumped by the closed timestamp or those who conflicted (write-read or write-write) with an existing future-time value.
      
      However, CockroachDB also supports a stricter model of consistency through its "linearizable" flag. When in linearizable mode (also known as "strict serializable" mode), all writing transactions (but not read-only transactions) must wait an additional max_offset after committing to ensure that their commit timestamp is below the current HLC clock time of any other node in the system. In doing so, all causally dependent transactions are guaranteed to start with higher timestamps, regardless of the gateway they use. This ensures that all causally dependent transactions commit with higher timestamps, even if their read and writes sets do not conflict with the original transaction's. This obviates the need for uncertainty intervals and prevents the "causal reverse" anamoly which can be observed by a third, concurrent transaction.
      
      For more, see https://www.cockroachlabs.com/blog/consistency-model/ and [docs/RFCS/20200811_non_blocking_txns.md](https://github.com/cockroachdb/cockroach/blob/master/docs/RFCS/20200811_non_blocking_txns.md).
      
      ----
      
      The PR also fixes a bug by properly marking read-only txn as aborted on rollback, which was missed in a85115a6.
      
      We were assuming that all calls to `commitReadOnlyTxnLocked` were for `EndTxn` requests with the Commit flag set to true, but this is not the case. This was not only confusing, but it was also leading to the `txn.commit` metric being incremented on rollback of a read-only transaction, instead of the `txn.aborts` metric.
      
      Release justification: New functionality.
      
      61170: kvserver: remove `kv.atomic_replication_changes.enabled` setting r=aayushshah15 a=aayushshah15
      
      This setting was added in 19.2 to provide a fallback against atomic
      replication changes. They've now been a part of CockroachDB for over 3
      releases. They're also a requirement for non-voting replicas.
      
      Release note (backward-incompatible change): Removed the
      `kv.atomic_replication_changes.enabled` cluster setting. All replication
      changes on a range now use joint-consensus.
      Co-authored-by: default avatarNathan VanBenschoten <[email protected]>
      Co-authored-by: default avatarAayush Shah <[email protected]>
      dcc8ab04
    • Nathan VanBenschoten's avatar
      kv: implement commit-wait for future time transaction commits · 7ec4212e
      Nathan VanBenschoten authored
      Fixes #57687.
      Related to #52745.
      
      This commit introduces a "commit-wait" sleep stage after a transaction
      commits, which is entered if doing so is deemed
      necessary for consistency.
      
      By default, commit-wait is only necessary for transactions that commit
      with a future-time timestamp that leads the local HLC clock. This is
      because CockroachDB's consistency model depends on all transactions
      waiting until their commit timestamp is below their gateway clock. In
      doing so, transactions ensure that at the time that they complete, all
      other clocks in the system (i.e. on all possible gateways) will be no
      more than the max_offset below the transaction's commit timestamp. This
      property ensures that all causally dependent transactions will have an
      uncertainty interval (see GlobalUncertaintyLimit) that exceeds the
      original transaction's commit timestamp, preventing stale reads. Without
      the wait, it would be possible for a read-write transaction to write a
      future-time value and then for a causally dependent transaction to read
      below that future-time value, violating "read your writes".
      
      The property must also hold for read-only transactions, which may have a
      commit timestamp in the future due to an uncertainty restart after
      observing a future-time value in their uncertainty interval. In such
      cases, the property that the transaction must wait for the local HLC
      clock to exceed its commit timestamp is not necessary to prevent stale
      reads, but it is necessary to ensure monotonic reads. Without the wait,
      it would be possible for a read-only transaction coordinated on a
      gateway with a fast clock to return a future-time value and then for a
      causally dependent read-only transaction coordinated on a gateway with a
      slow clock to read below that future-time value, violating "monotonic
      reads".
      
      In practice, most transactions do not need to wait at all, because their
      commit timestamps were pulled from an HLC clock (i.e. are not synthetic)
      and so they will be guaranteed to lead the local HLC's clock, assuming
      proper HLC time propagation. Only transactions whose commit timestamps
      were pushed into the future will need to wait, like those who wrote to a
      global_read range and got bumped by the closed timestamp or those who
      conflicted (write-read or write-write) with an existing future-time
      value.
      
      However, CockroachDB also supports a stricter model of consistency
      through its "linearizable" flag. When in linearizable mode (also known
      as "strict serializable" mode), all writing transactions (but not
      read-only transactions) must wait an additional max_offset after
      committing to ensure that their commit timestamp is below the current
      HLC clock time of any other node in the system. In doing so, all
      causally dependent transactions are guaranteed to start with higher
      timestamps, regardless of the gateway they use. This ensures that all
      causally dependent transactions commit with higher timestamps, even if
      their read and writes sets do not conflict with the original
      transaction's. This obviates the need for uncertainty intervals and
      prevents the "causal reverse" anamoly which can be observed by a third,
      concurrent transaction.
      
      For more, see https://www.cockroachlabs.com/blog/consistency-model/ and
      docs/RFCS/20200811_non_blocking_txns.md.
      
      Release notes: None
      
      Release justification: New functionality.
      7ec4212e
    • craig[bot]'s avatar
      Merge #61134 #61153 · 9595a158
      craig[bot] authored
      61134: sql: add distsql and vectorize settings to bundles r=RaduBerinde a=RaduBerinde
      
      Release justification: Low risk, high benefit changes to existing
      functionality.
      
      Release note: None
      
      61153: opt: show paired joiner continuation column r=RaduBerinde a=RaduBerinde
      
      Release justification: bug fixes and low-risk updates to new
      functionality.
      
      Release note: None
      Co-authored-by: default avatarRadu Berinde <[email protected]>
      9595a158
    • Nathan VanBenschoten's avatar
      kv/kvclient: properly mark read-only txn as aborted on rollback · 14a597a0
      Nathan VanBenschoten authored
      This was missed in a85115a6.
      
      We were assuming that all calls to `commitReadOnlyTxnLocked` were for
      `EndTxn` requests with the Commit flag set to true, but this is not the
      case. This was not only confusing, but it was also leading to the
      `txn.commit` metric being incremented on rollback of a read-only
      transaction, instead of the `txn.aborts` metric.
      
      Release justification: bug fix
      14a597a0
    • craig[bot]'s avatar
      Merge #61139 · 0a9c1639
      craig[bot] authored
      61139: sql: fixed information_schema.columns.udt_schema for enum types r=rafiss,arulajmani a=mnovelodou
      
      Previously, udt_schema displayed pg_catalog for any data type
      This was inadequate because user defined types may have a different schema
      To address this, this patch uses pg_catalog as default but checks for
      user defined type's schema if any.
      
      Release justification: bug fixes and low-risk updates to new functionality
      Release note (sql change): Fixed information_schema.columns.udt_schema for enum or
      user defined types
      
      Fixes #60145
      Co-authored-by: default avatarMiguelNovelo <[email protected]>
      0a9c1639
    • craig[bot]'s avatar
      Merge #61215 · 4d2d6c02
      craig[bot] authored
      61215: sql: deflake TestPGPreparedQuery r=RaduBerinde,rytaft a=rafiss
      
      A recent change to stats in SHOW TABLES made this test flaky. Stats are
      now read with AOST -10s by default, but for tests like this we need to
      modify this. See #60953 and https://github.com/cockroachdb/cockroach/pull/61191
      
      Release justification: test-only change
      
      Release note: None
      Co-authored-by: default avatarRafi Shamim <[email protected]>
      4d2d6c02
    • craig[bot]'s avatar
      Merge #61135 · 1427c6a2
      craig[bot] authored
      61135: sql: dropping and creating view in a single transaction behaves wrong. r=fqazi a=fqazi
      
      Fixes: #60737
      
      Previously, when dropping and creating a view/table/sequence with if
      exists clause in a single transaction would drop the view and not
      create the view again, since we would see a descriptor in a dropping
      state as created. Similarly, if the dropped object was a different
      type we would silently ignore the error for the IF NOT EXISTS case.
      To address this incorrect behaviour, this patch will return a
      ObjectNotInPrerequisiteState if a descriptor in a dropping state is
      found. Secondly, if the IF NOT EXISTS flag is specified we will
      validate that the types match otherwise return WrongObjectType.
      
        Release note (bug fix): Dropping and recreating a view/table/sequence
        in a transaction will now correctly error out if a conflicting object
        exists or if the drop is incomplete.
      
        Release justification: Low risk change to address a high severity
        issue where users could have failures due to incorrect behaviour.
      Co-authored-by: default avatarFaizan Qazi <[email protected]>
      1427c6a2
  3. 26 Feb, 2021 27 commits
    • Faizan Qazi's avatar
      sql: dropping and creating tables in a transaction behaves wrong · 0326d539
      Faizan Qazi authored
      Fixes: #60737
      
      Previously, when dropping and creating a view/table/sequence with if
      exists clause in a single transaction would drop the view and not
      create the view again, since we would see a descriptor in a dropping
      state as created. Similarly if the dropped object was a different
      type we would silently ignore the error for the IF NOT EXISTS case.
      To address this incorrect behaviour, this patch will return a
      ObjectNotInPrerequisiteState if a descriptor in a dropping state is
      found. Secondly, if the IF NOT EXISTS flag is specified we will
      validate that the types match otherwise return WrongObjectType.
      
      Release note (bug fix): Dropping and recreating a view/table/sequence
      in a transaction will now correctly error out if a conflicting object
      exists or if the drop is incomplete.
      
      Release justification: Low risk change to address a high severity
      issue where users could have failures due to incorrect behaviour.
      0326d539
    • craig[bot]'s avatar
      Merge #60461 #61184 · cc3ecaa9
      craig[bot] authored
      60461:  backupccl: restore zone configs first r=dt a=pbardea
      
      This commit breaks down cluster restores into several phases. The first
      restores the zone config table, and the second restores the rest of the
      data.
      
      It introduces a struct that specifies the data to be restored in one
      instance of the restoration flow. Each phase creates its own restoration
      flow, and each phase should be idempontent.
      
      Release justification: High impact change, zone configs are now
      respected during a restore.
      
      Release note (sql change): cluster backup now restores the zone
      configurations first. This means that there should be less
      range-relocation during and after cluster restores.
      
      61184: opt: remove unique checks for constraints used as arbiters r=rytaft a=rytaft
      
      This commit removes uniqueness checks for `UNIQUE WITHOUT INDEX`
      constraints that are already used as arbiters to detect conflicts
      in `INSERT ... ON CONFLICT` statements. Unique checks are not needed
      in this case to enforce uniqueness.
      
      Fixes #59119
      
      Release justification: This commit is a low-risk update to new
      functionality.
      
      Release note (performance improvement): The optimizer no longer plans
      uniqueness checks for columns in implicitly partitioned unique indexes
      when those columns are used as the arbiters to detect conflicts
      in INSERT ... ON CONFLICT statements. Unique checks are not needed
      in this case to enforce uniqueness. Removing these checks results in
      improved performance for INSERT ... ON CONFLICT statements.
      Co-authored-by: default avatarPaul Bardea <[email protected]>
      Co-authored-by: default avatarRebecca Taft <[email protected]>
      cc3ecaa9
    • Rafi Shamim's avatar
      sql: deflake TestPGPreparedQuery · d6a562a7
      Rafi Shamim authored
      A recent change to stats in SHOW TABLES made this test flaky. Stats are
      now read with AOST -10s by default, but for tests like this we need to
      modify this.
      
      Release justification: test-only change
      
      Release note: None
      d6a562a7
    • MiguelNovelo's avatar
      sql: fixed information_schema.columns.udt_schema for enum types · e3b8dfcf
      MiguelNovelo authored
      Previously, udt_schema displayed pg_catalog for any data type
      This was inadequate because user defined types may have a different schema
      To address this, this patch uses pg_catalog as default but checks for
      user defined type's schema if any.
      
      Release justification: bug fixes and low-risk updates to new functionality
      Release note (sql change): Fixed information_schema.columns.udt_schema for enum or
      user defined types
      
      Fixes #60145
      e3b8dfcf
    • craig[bot]'s avatar
      Merge #60744 #60952 · de72eae2
      craig[bot] authored
      60744: sql: drop default value when its dependent sequence is dropped r=the-ericwang35 a=the-ericwang35
      
      Fixes https://github.com/cockroachdb/cockroach/issues/51889 and https://github.com/cockroachdb/cockroach/issues/20965.
      
      Previously, we did not support `DROP SEQUENCE ... CASCADE`,
      and we simply disallowed dropping sequences that have usages by 
      tables when a user tries to drop a sequence.
      
      However,  we did not make this check when doing 
      `DROP DATABASE/SCHEMA ... CASCADE`. As a result,
      if a sequence is used by a table in a different database/schema,
      we end up dropping a sequence that the table still relies on,
      resulting in a corrupted default expression.
      
      This patch addresses this issue by implementing 
      `DROP SCHEMA ... CASCADE`, which solves the above issue.
      When we drop anything that contains/owns a sequence with
      `CASCADE`, we drop those sequences as if 
      `DROP SCHEMA ... CASCADE` was called on them, which
      prevents the `DEFAULT` expressions from being corrupted, 
      and also allows users to use `DROP SEQUENCE ... CASCADE.
      
      `DROP SEQUENCE ... CASCADE` behaves as in Postgres, 
      wherein any default expressions that rely on the dropped 
      sequences are also dropped.
      
      Release note (bug fix): Fixed a bug which could result in corrupted descriptors when dropping a database which contains a sequence that was referenced from another database.
      
      Release note (bug fix): Fixed a bug where DROP SEQUENCE CASCADE would not properly remove references to itself from table default expressions.
      
      Release note (sql change): Added support for dropping a table which owns a sequence. 
      
      Release justification: low risk bug fix for existing functionality
      
      60952: server: Migrate nodes, {hot-,}ranges, health endpoints to v2 API r=knz a=itsbilal
      
      This change adds API v2 compatible versions of the node list, range info
      per node, ranges info across nodes, hot ranges, and health endpoints.
      These endpoints all support API v2 header-based authentication,
      pagination (if applicable), and only return relevant information
      in the response payloads.
      
      One part of https://github.com/cockroachdb/cockroach/issues/55947.
      
      Release note (api change): Add these new HTTP API endpoints:
       - `/api/v2/nodes/`: Lists all nodes in the cluster
       - `/api/v2/nodes/<node_id>/ranges`: Lists all ranges on the specified node
       - `/api/v2/ranges/hot/`: Lists hot ranges in the cluster
       - `/api/v2/ranges/<range_id>/`: Describes range in more detail
       - `/api/v2/health/`: Returns an HTTP 200 response if node is healthy.
      
      Release justification: Adds more HTTP API endpoints in parallel
       that do not touch existing code.
      Co-authored-by: default avatarEric Wang <[email protected]>
      Co-authored-by: default avatarBilal Akhtar <[email protected]>
      de72eae2
    • Rebecca Taft's avatar
      opt: remove unique checks for constraints used as arbiters · 06c6a646
      Rebecca Taft authored
      This commit removes uniqueness checks for UNIQUE WITHOUT INDEX
      constraints that are already used as arbiters to detect conflicts
      in INSERT ... ON CONFLICT statements. Unique checks are not needed
      in this case to enforce uniqueness.
      
      Fixes #59119
      
      Release justification: This commit is a low-risk update to new
      functionality.
      
      Release note (performance improvement): The optimizer no longer plans
      uniqueness checks for columns in implicitly partitioned unique indexes
      when those columns are used as the arbiters to detect conflicts
      in INSERT ... ON CONFLICT statements. Unique checks are not needed
      in this case to enforce uniqueness. Removing these checks results in
      improved performance for INSERT ... ON CONFLICT statements.
      06c6a646
    • craig[bot]'s avatar
      Merge #61195 · be0383e1
      craig[bot] authored
      61195: opt: fix inverted join logical prop output cols r=mgartner a=mgartner
      
      An inverted join may project a subset of its input columns. This was not
      accounted for when building logical properties, which led caused test
      build assertions to fail. This commit corrects the issue.
      
      Fixes #59285
      
      Release justification: This is a low-risk bug fix for inverted joins.
      
      Release note: None
      Co-authored-by: default avatarMarcus Gartner <[email protected]>
      be0383e1
    • Radu Berinde's avatar
      opt: show paired joiner continuation column · b5bbdf44
      Radu Berinde authored
      Release justification: bug fixes and low-risk updates to new
      functionality.
      
      Release note: None
      b5bbdf44
    • Radu Berinde's avatar
      sql: add distsql and vectorize settings to bundles · ed739fca
      Radu Berinde authored
      Add distsql and vectorize session settings to bundles, and also add
      the current value of all cluster settings.
      
      Release justification: Low risk, high benefit changes to existing
      functionality.
      
      Release note: None
      ed739fca
    • Bilal Akhtar's avatar
      server: Migrate nodes, {hot-,}ranges, health endpoints to v2 API · 7c55798b
      Bilal Akhtar authored
      This change adds API v2 compatible versions of the node list, range info
      per node, ranges info across nodes, hot ranges, and health endpoints.
      These endpoints all support API v2 header-based authentication,
      pagination (if applicable), and only return relevant information
      in the response payloads.
      
      Release note (api change): Add these new HTTP API endpoints:
       - `/api/v2/nodes/`: Lists all nodes in the cluster
       - `/api/v2/nodes/<node_id>/ranges`: Lists all ranges on the specified node
       - `/api/v2/ranges/hot/`: Lists hot ranges in the cluster
       - `/api/v2/ranges/<range_id>/`: Describes range in more detail
       - `/api/v2/health/`: Returns an HTTP 200 response if node is healthy.
      
      Release justification: Adds more HTTP API endpoints in parallel
       that do not touch existing code.
      7c55798b
    • Radu Berinde's avatar
      kv: disable race testing of kvserver package · ae797e41
      Radu Berinde authored
      The thread sanitizer dies with a "clock allocator overflow" on this
      package. The failure is not specific to a test; we have skipped the
      tests that were running and it's now being hit during another test.
      
      I tried splitting out the client tests, but they rely on a lot of
      _test code from the `kvserver` package. It requires duplicating or
      moving around a lot of code (and making many methods public from
      non-test kvserver code).
      
      Until we figure out a solution, I am skipping the entire package under
      race.
      
      Informs #61120.
      
      Release justification: non-production code change.
      
      Release note: None
      ae797e41
    • craig[bot]'s avatar
      Merge #61159 · c504ae70
      craig[bot] authored
      61159: sql: fix reverting schema changes for databases and schemas r=lucy-zhang a=lucy-zhang
      
      Previously, we were failing to handle the case where a schema change job
      to drop or rename a database or schema entered the reverting state. In
      `OnFailOrCancel`, we were always trying to look up the descriptor
      associated with the job assuming it was a table. This led to a panic in
      20.2. On master we get return an internal error `relation [id] not
      found`.
      
      We also had the preexisting behavior that when dropping a database, we
      would return nil from `OnFailOrCancel` without indicating anything was
      wrong. This is undesirable, since it's abnormal for these jobs to enter
      the reverting state, and we can't actually either revert the schema
      change or clean up.
      
      This PR adds checks at the top of `OnFailOrCancel` so that we now exit
      early with an error if the descriptor is not a table, and makes the
      behavior for all database and schema jobs consistent.
      
      Fixes #59415.
      
      Release justification: Fixes for high-priority or high-severity bugs in
      existing functionality
      
      Release note (bug fix): Fixed a bug where schema changes on databases
      and schemas could return a `relation [<id>] does not exist` if they
      failed or were canceled and entered the reverting state. These jobs are
      not actually possible to revert. With this change, the correct error
      causing the job to fail will be returned, and the job will enter the
      failed state with an error indicating that the job could not be
      reverted.
      Co-authored-by: default avatarLucy Zhang <[email protected]>
      c504ae70
    • craig[bot]'s avatar
      Merge #61073 · eb0aa060
      craig[bot] authored
      61073: sql: update database zone configurations after region drop finalization r=otan a=arulajmani
      
      Previously, database level zone configurations weren't updated when the
      region drop was finalized. This patch adds that support.
      
      This patch also moves setting zone configurations on ADD REGION from
      the user txn to the type schema changer. This ensures that zone
      configurations are added transactionally -- if the ADD REGION fails for
      whatever reason, we no longer leave behind dangling zone config.
      
      Lastly, I've refactored some test setup code that was being duplicated
      and improved the tests around rollback to test additional scenarios.
      
      Closes #60435
      Closes #60750
      
      Release note: None
      
      Release justification: bug fixes and low-risk updates to new
      functionality
      Co-authored-by: default avatararulajmani <[email protected]>
      eb0aa060
    • Eric Wang's avatar
      sql: drop default value when its dependent sequence is dropped · c02dd6fa
      Eric Wang authored
      Fixes https://github.com/cockroachdb/cockroach/issues/51889.
      
      Previously, when we drop a database or a schema, we do not check
      if it contains any sequences that are used by anything in
      other databases/schemas. As a result, if a sequence is
      used by a table in a different database/schema, we end up
      dropping a sequence that the tables still relies on, resulting
      in a corrupted default expression.
      This patch addresses this issue by emulating Postgres behavior
      by removing the default value from any columns that
      use the dropped sequence.
      
      Release note (bug fix): drop default value when its dependent
      sequence is dropped.
      
      Release justification: low risk bug fix for existing functionality
      c02dd6fa
    • craig[bot]'s avatar
      Merge #61162 #61191 · 45deb66a
      craig[bot] authored
      61162: sql: add pg_class.relpartbound column r=RichardJCai a=rafiss
      
      DBeaver relies on this column existing.
      
      Release note (sql change): The pg_catalog.pg_class table now has a
      relpartbound column. This is only for compatibility, and the column
      value is always NULL.
      
      Release justification: Low-risk, high-benefit change to existing
      functionality.
      
      61191: sql: fix SHOW TABLES FROM db stats display r=RichardJCai a=rafiss
      
      fixes #58652
      
      Release justification: low-risk bug fix to existing functionality.
      
      Release note (bug fix): The `SHOW TABLES FROM database` command would
      always show a NULL estimated_row_count if inspecting a database that was
      not the current database. This is now fixed.
      Co-authored-by: default avatarRafi Shamim <[email protected]>
      45deb66a
    • Marcus Gartner's avatar
      opt: fix inverted join logical prop output cols · 9a265005
      Marcus Gartner authored
      An inverted join may project a subset of its input columns. This was not
      accounted for when building logical properties, which led caused test
      build assertions to fail. This commit corrects the issue.
      
      Fixes #59285
      
      Release justification: This is a low-risk bug fix for inverted joins.
      
      Release note: None
      9a265005
    • craig[bot]'s avatar
      Merge #61028 · c943b6d0
      craig[bot] authored
      61028: sql: validate partial unique constraints r=mgartner a=mgartner
      
      Validation for partial unique constraints now takes into account the
      constraint's predicate expression. This allows creating implicitly
      partitioned unique partial indexes on tables with duplicate unique
      column values for rows that do not satisfy the predicate.
      
      Release note: None
      
      Release justification: Correct validation of partial unique constraints
      is required for supporting implicitly partitioned partial unique
      indexes.
      Co-authored-by: default avatarMarcus Gartner <[email protected]>
      c943b6d0
    • Bilal Akhtar's avatar
      server: Small tweaks to offset-based pagination code · 40f58aa6
      Bilal Akhtar authored
      Makes small changes to simplePaginate such as returning
      a 0 for offset if the end of a slice has been reached. This
      works well with `omitzero` JSON fields as that'd let the Next
      value go ignored.
      
      Also adds a getSimplePaginateValues to parse request query string
      values for offset-based (aka "simple") pagination.
      
      Release note: None.
      
      Release justification: Small, low-risk change that only affects
       new endpoints that exist in parallel to existing ones.
      40f58aa6
    • Rafi Shamim's avatar
      sql: fix SHOW TABLES FROM db stats display · fe801c77
      Rafi Shamim authored
      Release justification: low-risk bug fix to existing functionality.
      
      Release note (bug fix): The `SHOW TABLES FROM database` command would
      always show a NULL estimated_row_count if inspecting a database that was
      not the current database. This is now fixed.
      fe801c77
    • craig[bot]'s avatar
      Merge #60693 · 11d37f5f
      craig[bot] authored
      60693: sql: audit all usages of QueryEx to use iterator pattern r=yuzefovich a=yuzefovich
      
      This commit audits the usage of `QueryEx` method of the internal
      executor in the following manner:
      - if the caller only needed to execute the statement, `ExecEx` is now
      used
      - if the query can return at most one row, then `QueryRowEx` is now used
      - if the caller can be refactored to use the iterator pattern, it is
      done so.
      
      As a result, almost all usages have been refactored (most notably the
      virtual `crdb_internal.jobs` table now uses the iterator pattern, thus
      aleviating OOM concerns - the ad-hoc memory accounting logic has been
      removed).
      
      `QueryEx` has been renamed to `QueryBufferedEx` to highlight that the
      full buffering occurs, and it was added to `sqlutil.InternalExecutor`
      interface. The method is now used only in two places.
      
      Addresses: #48595.
      
      Release justification: bug fix.
      
      Release note (bug fix): `crdb_internal.jobs` virtual table is now
      populated in a paginated fashion, thus, alleviating memory related
      concerns when previously we could encounter OOM crash.
      Co-authored-by: default avatarYahor Yuzefovich <[email protected]>
      11d37f5f
    • arulajmani's avatar
      sql: update database zone configurations after region drop finalization · bd6d7899
      arulajmani authored
      Previously, database level zone configurations weren't updated when the
      region drop was finalized. This patch adds that support.
      
      This patch also moves setting zone configurations on ADD REGION from
      the user txn to the type schema changer. This ensures that zone
      configurations are added transactionally -- if the ADD REGION fails for
      whatever reason, we no longer leave behind dangling zone config.
      
      Lastly, I've refactored some test setup code that was being duplicated
      and improved the tests around rollback to test additional scenarios.
      
      Closes #60435
      Closes #60750
      
      Release note: None
      
      Release justification: bug fixes and low-risk updates to new
      functionality
      bd6d7899
    • Yahor Yuzefovich's avatar
      sql: audit all usages of QueryEx to use iterator pattern · dbc86766
      Yahor Yuzefovich authored
      This commit audits the usage of `QueryEx` method of the internal
      executor in the following manner:
      - if the caller only needed to execute the statement, `ExecEx` is now
      used
      - if the query can return at most one row, then `QueryRowEx` is now used
      - if the caller can be refactored to use the iterator pattern, it is
      done so.
      
      As a result, almost all usages have been refactored (most notably the
      virtual `crdb_internal.jobs` table now uses the iterator pattern, thus
      aleviating OOM concerns - the ad-hoc memory accounting logic has been
      removed).
      
      `QueryEx` has been renamed to `QueryBufferedEx` to highlight that the
      full buffering occurs, and it was added to `sqlutil.InternalExecutor`
      interface. The method is now used only in three places.
      
      Release justification: bug fix.
      
      Release note (bug fix): `crdb_internal.jobs` virtual table is now
      populated in a paginated fashion, thus, alleviating memory related
      concerns when previously we could encounter OOM crash.
      dbc86766
    • craig[bot]'s avatar
      Merge #61151 · 9902ea9c
      craig[bot] authored
      61151: build: show a warning if you're using an old version of `make` r=rickystewart a=rickystewart
      
      Today, some people started noticing `protoc` build failures on old
      versions of `make`. Insert a warning to urge people to upgrade.
      
      Release justification: non-production code changes
      Release note: None
      Co-authored-by: default avatarRicky Stewart <[email protected]>
      9902ea9c
    • Paul Bardea's avatar
      backupccl: restore zone configs first · f4e6c2cd
      Paul Bardea authored
      This commit breaks down cluster restores into several phases. The first
      restores the zone config table, and the second restores the rest of the
      data.
      
      It introduces a struct that specifies the data to be restored in one
      instance of the restoration flow. Each phase creates its own restoration
      flow, and each phase should be idempontent.
      
      Release justification: High impact change, zone configs are now
      respected during a restore.
      
      Release note (sql change): cluster backup now restores the zone
      configurations first. This means that there should be less
      range-relocation during and after cluster restores.
      f4e6c2cd
    • craig[bot]'s avatar
      Merge #61187 · 7df2237d
      craig[bot] authored
      61187: bulkio: Correctly check stream replication feature gate. r=miretskiy a=miretskiy
      
      Verify experimental stream replication feature is enabled only
      after making sure that the statement we're attempting to execute
      is a replication stream statement.
      
      Release Justification: minor bug fix.
      Release Notes: None
      Co-authored-by: default avatarYevgeniy Miretskiy <[email protected]>
      7df2237d
    • craig[bot]'s avatar
      Merge #60742 · ed151122
      craig[bot] authored
      60742: importccl: upgrade Vitess MySQL parser to v8.0.0 r=dt,adityamaru a=pbardea
      
      This commit upgrades the parser used by IMPORT when parsing MySQL files
      to v8.0.0. This change points to a new branch in our Vitess fork that
      has been rebased on v8.0.0.
      
      We maintain a fork of this repository with some custom patches. See
      custom commits that have diverged from the release commit in that repo
      for patch details.
      
      Old: https://github.com/cockroachdb/vitess/tree/no-flag
      New: https://github.com/cockroachdb/vitess/tree/v8.0.0-cockroach
      
      Vitess releases: https://github.com/vitessio/vitess/releases
      
      Release justification: high impact. Our MySQL processor hasn't been
      updated in a long time, it's better to get this in so it can bake during
      stability.
      
      Release note: None
      Co-authored-by: default avatarPaul Bardea <[email protected]>
      ed151122
    • Yevgeniy Miretskiy's avatar
      bulkio: Correctly check stream replication feature gate. · 496b012d
      Yevgeniy Miretskiy authored
      Verify experimental stream replication feature is enabled only
      after making sure that the statement we're attempting to execute
      is a replication stream statement.
      
      Release Justification: minor bug fix.
      Release Notes: None
      496b012d