This project is mirrored from https://github.com/cockroachdb/cockroach.git. Pull mirroring updated .
  1. 27 Jan, 2023 1 commit
  2. 26 Jan, 2023 33 commits
    • Marcus Gartner's avatar
      sql/logictest: add UDF tests with tagged dollar quotes · f377adc6
      Marcus Gartner authored
      Epic: None
      
      Release note: None
      f377adc6
    • craig[bot]'s avatar
      Merge #95839 #95900 #96005 #96008 #96009 #96020 · 32b53450
      craig[bot] authored
      
      
      95839: colinfo: add missing type families into CanHaveCompositeKeyEncoding r=yuzefovich a=yuzefovich
      
      This commit adds several missing type families into `CanHaveCompositeKeyEncoding` method. Some of these type families are internal and, probably, don't need to be handled, but we recently introduced TSVector and TSQuery types which were incorrectly marked as being composite since they were not mentioned explicitly in the type family switch. This commit also makes it so that we panic in this method if we forget to include a newly-introduced type into this switch.
      
      Fixes: #95680.
      
      Release note: None
      
      95900: ui: add check for cpu usage r=maryliag a=maryliag
      
      Add a check, for cases when the value might no be
      returned (cluster with mixed versions).
      
      Part Of #87213
      
      Release note: None
      
      96005: colfetcher: disable direct columnar scans for now r=yuzefovich a=yuzefovich
      
      This commit disables direct columnar scans which are now randomly enabled in tests due to this feature having a data race at the moment.
      
      Informs: #95937.
      
      Release note: None
      
      96008: roachtest/awsdms: fix race condition that can cause panics r=otan a=Jeremyyang920
      
      In #95518, we added a new test case to check for a table error during the DMS process. However there is a race when the check happens depending on how quickly tasks have ran and the task ReplicationTaskStats can be nil and we tried to access field on the nil object that was returned from the API call.
      
      This commit checks that the ReplicationTaskStat is not nil before accessing the TablesErrored property.
      
      Fixes: #93305
      Release note: None
      
      96009: binfetcher: fix binary downloading for arm64 MacOS r=rail a=andyyang890
      
      This patch updates the suffix used to fetch arm64 MacOS binaries
      to match the naming scheme used to build releases.
      
      Epic: None
      
      Release note: None
      
      96020: roachtest: use teardown log when creating GitHub issue r=herkolategan a=renatolabs
      
      This is a follow up of #95831. The logger passed to the `githubIssues`
      struct writes to the test runner logger, which is not ideal. This
      changes the logger passed to use the `teardown` logger, so log entries
      related to GitHub issue creation are in the same directory as the
      failing test itself.
      
      Epic: None
      
      Release note: None
      
      Co-authored-by: default avatarYahor Yuzefovich <yahor@cockroachlabs.com>
      Co-authored-by: default avatarmaryliag <marylia@cockroachlabs.com>
      Co-authored-by: default avatarJeremy Yang <jyang@cockroachlabs.com>
      Co-authored-by: default avatarAndy Yang <yang@cockroachlabs.com>
      Co-authored-by: default avatarRenato Costa <renato@cockroachlabs.com>
      32b53450
    • craig[bot]'s avatar
      Merge #94153 #95855 #95861 #95876 #95997 #95998 #96003 · ed9928ff
      craig[bot] authored
      94153: sql: Remove type annotations from `column_default` in `information_schema.columns` r=ZhouXing19 a=e-mbrown
      
      Informs: https://github.com/cockroachdb/cockroach/issues/87774
      
      The type annotation on `column_default` caused confusion in postgres compatible apps. This change formats the `column_default` without the type annotation. It does not address the incorrect type annotation.
      
      Release note (bug fix): The content of `column_default` in `information_schema.columns` no longer have type annotations.
      
      95855: backupccl: run restore/tpce/32tb/aws/nodes=15/cpus=16 once a week r=benbardin a=msbutler
      
      This patch adds the restore/tpce/32TB/aws/nodes=15/cpus=16 test to our weekly
      test suite. The backup fixture was generated by initing a tpce cluster with 2
      Million customers, and running this workload while 48 incremental backups were
      taken at 15 minute increments. The roachtest restores from a system time
      captured in the 24th backup.
      
      The cluster used to restore the backup will run on aws with 15 nodes each with
      16 vcpus.
      
      To verify this test exists, run:
      `roachtest list tag:weekly`
      
      Release note: None
      
      Epic: none
      
      95861: kv: remove lastToReplica and lastFromReplica from raftMu r=nvanbenschoten a=nvanbenschoten
      
      Extracted from #94165 without modification.
      
      ----
      
      In 410ef299, we moved these fields from under the Replica.mu to under the Replica.raftMu. This was done to avoid lock contention.
      
      In this commit, we move these fields under their own mutex so that they can be accessed without holding the raftMu. This allows us to send Raft messages from other goroutines.
      
      The commit also switches from calling RawNode.ReportUnreachable directly in sendRaftMessage to using the more flexible unreachablesMu set, which defers the call to ReportUnreachable until the next Raft tick. As a result, the commit closes #84246.
      
      Release note: None
      Epic: None
      
      95876: sql: remove round-trips from common DISCARD ALL r=ajwerner a=ajwerner
      
      #### sql: avoid checking if a role exists when setting to the current role
      
      This is an uncached round-trip. It makes `DISCARD` expensive.
      In https://github.com/cockroachdb/cockroach/pull/86485
      
       we implemented
      `SET SESSION AUTHORIZATION DEFAULT`, this added an extra sql query to
      `DISCARD ALL` to check if the role we'd become exists. This is almost
      certainly unintentional in the case where the role we'd become is the
      current session role. I think this just happened because of code
      consolidation.
      
      We now no longer check if the current session role exists. This removes
      the last round-trip from DISCARD ALL.
      
      #### sql: use in-memory session data to decide what to do in DISCARD
      
      In #86246 we introduced logic to discard schemas when running DISCARD ALL and DISCARD TEMP. This logic did expensive kv operations unconditionally; if the session knew it had never created a temporary schema, we'd still fetch all databases and proceed to search all databases for a temp schema. This is very expensive.
      
      Fixes: #95864
      
      Release note (performance improvement): In 22.2, logic was added to make
      `SET SESSION AUTHORIZATION DEFAULT` not a no-op. This implementation used
      more general code for setting the role for a session which made sure that
      the role exists. The check for whether a role exists is currently uncached.
      We don't need to check if the role we already are exists. This improves the
      performance of `DISCARD ALL` in addition to `SET SESSION AUTHORIZATION
      DEFAULT`.
      
      
      Release note (performance improvement): In 22.2, we introduced support for `DISCARD TEMP` and made `DISCARD ALL` actually discard temp tables. This implementation ran expensive logic to discover temp schemas rather than consulting in-memory data structures. As a result, `DISCARD ALL`, which is issued regularly by connection pools, became an expensive operation when it should be cheap. This problem is now resolved.
      
      95997: ui: hide list of statement for non-admins r=maryliag a=maryliag
      
      To get the list of fingerprints used by an index, we use the `system.statement_statistics` directly, but non-admins don't have access to system table.
      Until #95756 or #95770 are done, we need to hide
      this feature for non-admins.
      
      Part Of #93087
      
      Release note (ui change): Hide list of used fingerprint per index on Index Details page, for non-admin users.
      
      95998: roachtest: update activerecord blocklist r=ZhouXing19 a=andyyang890
      
      This patch removes a few tests from the blocklist that do not
      exist in the test suite for rails 7.0.3, which were being run
      prior to the fix for version pinning.
      
      Informs #94211
      
      Release note: None
      
      96003: kv: add MVCC local timestamp details to ReadWithinUncertaintyIntervalError r=nvanbenschoten a=nvanbenschoten
      
      This PR adds details about MVCC local timestamps to ReadWithinUncertaintyIntervalErrors. This provides more information about the cause of uncertainty errors.
      
      The PR also includes a series of minor refactors to clean up this code.
      
      Release note: None
      Epic: None
      
      Co-authored-by: default avatare-mbrown <ebsonari@gmail.com>
      Co-authored-by: default avatarMichael Butler <butler@cockroachlabs.com>
      Co-authored-by: default avatarNathan VanBenschoten <nvanbenschoten@gmail.com>
      Co-authored-by: Andrew Werner's avatarAndrew Werner <awerner32@gmail.com>
      Co-authored-by: default avatarmaryliag <marylia@cockroachlabs.com>
      Co-authored-by: default avatarAndy Yang <yang@cockroachlabs.com>
      ed9928ff
    • craig[bot]'s avatar
      Merge #96023 · 831dbcc2
      craig[bot] authored
      
      
      96023: multiregion: skip TestColdStartLatency r=ajwerner a=msbutler
      
      Epic: none
      
      Informs: #95644
      
      Co-authored-by: default avatarMichael Butler <butler@cockroachlabs.com>
      831dbcc2
    • Michael Butler's avatar
      multiregion: skip TestColdStartLatency · 4e6aaf38
      Michael Butler authored
      Epic: none
      
      Informs: #95644
      4e6aaf38
    • Renato Costa's avatar
      roachtest: use teardown log when creating GitHub issue · dbc8bfd8
      Renato Costa authored
      This is a follow up of #95831. The logger passed to the `githubIssues`
      struct writes to the test runner logger, which is not ideal. This
      changes the logger passed to use the `teardown` logger, so log entries
      related to GitHub issue creation are in the same directory as the
      failing test itself.
      
      Release note: None
      dbc8bfd8
    • craig[bot]'s avatar
      Merge #95523 #95748 · 705d6a1b
      craig[bot] authored
      
      
      95523: insights: record insights when txn contention meets threshold r=xinhaoz a=xinhaoz
      
      Part of #90393
      
      Previously, we only write a transaction and its statements to the insights system when there is an issue detected for one of the statements in the transaction. However, a transaction should still report high contention when the total amount of contention time is high, even if the contention experienced by each of its statements is not over the threshold.
      
      This commit ensures that transactions which have a recorded contention time above the insights latency threshold get reported to the insights system.
      
      Release note: None
      
      95748: raftlog: introduce EntryEncoding{Standard,Sideloaded}WithAC r=irfansharif a=irfansharif
      
      Part of #95563. Predecessor to #95637.
      
      This commit introduces two new encodings for raft log entries, `EntryEncoding{Standard,Sideloaded}WithAC`. Raft log entries have prefix byte that informs decoding routines how to interpret the subsequent bytes. To date we've had two, `EntryEncoding{Standard,Sideloaded}`[^1], to indicate whether the entry came with sideloaded data[^2]. Our two additions here will be used to indicate whether the particular entry is subject to replication admission control. If so, right as we persist entries into the raft log storage, we'll "admit the work without blocking", which is further explained in #95637.
      
      The decision to use replication admission control happens above raft and a per-entry basis. If using replication admission control, AC-specific metadata will be plumbed down as part of the marshaled raft command. This too is explained in in #95637, specifically, the 'RaftAdmissionMeta' section. This commit then adds an unused version gate (`V23_1UseEncodingWithBelowRaftAdmissionData`) to use replication admission control. Since we're using a different prefix byte for raft commands, one not recognized in earlier CRDB versions, we need explicit versioning. We add it out of development convenience -- adding version gates is most prone to merge conflicts. We expect to use it shortly, before alpha/beta cuts.
      
      [^1]: Now renamed to `EntryEncoding{Standard,Sideloaded}WithoutAC`.
      [^2]: These are typically AddSSTs, the storage for which is treated differently for performance reasons.
      
      Release note: None
      
      Co-authored-by: default avatarXin Hao Zhang <xzhang@cockroachlabs.com>
      Co-authored-by: Irfan Sharif's avatarirfan sharif <irfanmahmoudsharif@gmail.com>
      705d6a1b
    • Irfan Sharif's avatar
      raftlog: introduce EntryEncoding{Standard,Sideloaded}WithAC · 4df47f54
      Irfan Sharif authored
      Part of #95563. Predecessor to #95637.
      
      This commit introduces two new encodings for raft log entries,
      EntryEncoding{Standard,Sideloaded}WithAC. Raft log entries have prefix
      byte that informs decoding routines how to interpret the subsequent
      bytes. To date we've had two, EntryEncoding{Standard,Sideloaded}[^1], to
      indicate whether the entry came with sideloaded data[^2]. Our two
      additions here will be used to indicate whether the particular entry is
      subject to replication admission control. If so, right as we persist
      entries into the raft log storage, we'll "admit the work without
      blocking", which is further explained in #95637.
      
      The decision to use replication admission control happens above raft and
      a per-entry basis. If using replication admission control, AC-specific
      metadata will be plumbed down as part of the marshaled raft command.
      This too is explained in in #95637, specifically, the
      'RaftAdmissionMeta' section. When using these encodings in the future,
      we'll need to tied it to a version gate since we're using a prefix byte
      for raft commands one that's recognized in earlier CRDB versions.
      
      [^1]: Now renamed to EntryEncoding{Standard,Sideloaded}WithoutAC.
      [^2]: These are typically AddSSTs, the storage for which is treated
            differently for performance reasons.
      
      Release note: None
      4df47f54
    • Jeremy Yang's avatar
      roachtest/awsdms: fix race condition that can cause panics · 3ef8fbbd
      Jeremy Yang authored
      In #95518, we added a new test case to check for a table error
      during the DMS process. However there is a race when the check
      happens depending on how quickly tasks have ran and the task
      ReplicationTaskStats can be nil and we tried to access field
      on the nil object that was returned from the API call.
      
      This commit checks that the ReplicationTaskStat is not nil
      before accessing the TablesErrored property.
      
      Fixes: #93305
      Release note: None
      3ef8fbbd
    • Andy Yang's avatar
      binfetcher: fix binary downloading for arm64 MacOS · 2ac04f71
      Andy Yang authored
      This patch updates the suffix used to fetch arm64 MacOS binaries
      to match the naming scheme used to build releases.
      
      Release note: None
      2ac04f71
    • Nathan VanBenschoten's avatar
      kv: remove lastToReplica and lastFromReplica from raftMu · 1fff7819
      Nathan VanBenschoten authored
      In 410ef299, we moved these fields from under the Replica.mu to under the
      Replica.raftMu. This was done to avoid lock contention.
      
      In this commit, we move these fields under their own mutex so that they
      can be accessed without holding the raftMu. This allows us to send Raft
      messages from other goroutines.
      
      The commit also switches from calling RawNode.ReportUnreachable directly
      in sendRaftMessage to using the more flexible unreachablesMu set, which
      defers the call to ReportUnreachable until the next Raft tick. As a result,
      the commit closes #84246.
      
      Release note: None
      Epic: None
      1fff7819
    • Nathan VanBenschoten's avatar
      kv: add MVCC local timestamp details to ReadWithinUncertaintyIntervalError · 812c5b95
      Nathan VanBenschoten authored
      This commit adds details about MVCC local timestamps to ReadWithinUncertaintyIntervalErrors.
      This provides more information about the cause of uncertainty errors.
      
      Release note: None
      Epic: None
      812c5b95
    • Yahor Yuzefovich's avatar
      colfetcher: disable direct columnar scans for now · 1786abfe
      Yahor Yuzefovich authored
      This commit disables direct columnar scans which are now randomly
      enabled in tests due to this feature having a data race at the moment.
      
      Release note: None
      1786abfe
    • craig[bot]'s avatar
      Merge #95862 · 3b5bcf07
      craig[bot] authored
      
      
      95862: storage: add CommitNoSyncWait and SyncWait to Batch interface r=sumeerbhola a=nvanbenschoten
      
      Extracted from #94165.
      
      Picks up cockroachdb/pebble/pull/2117.
      
      Release note: None
      Epic: None
      
      Co-authored-by: default avatarNathan VanBenschoten <nvanbenschoten@gmail.com>
      3b5bcf07
    • e-mbrown's avatar
      sql: Remove type annotations from `column_default` in `information_schema · bc3fccd2
      e-mbrown authored
      .columns`
      
      The type annotation on `column_default` caused confusion
      in postgres compatible apps. This change formats the
      `column_default` without the type annotation. It does not
      address the incorrect type annotation.
      
      Release note (bug fix): The content of `column_default`
      in `information_schema.columns` no longer have type
      annotations.
      bc3fccd2
    • Nathan VanBenschoten's avatar
      kv: group reader and value info in ReadWithinUncertaintyIntervalError · 533e6045
      Nathan VanBenschoten authored
      Code movement.
      
      Release note: None
      Epic: None
      533e6045
    • Nathan VanBenschoten's avatar
      kv: retype ReadWithinUncertaintyIntervalError.LocalUncertaintyLimit as ClockTimestamp · ac50977d
      Nathan VanBenschoten authored
      Type check to avoid unnecessary upcasting. Improves type safety.
      
      Release note: None
      Epic: None
      ac50977d
    • Nathan VanBenschoten's avatar
      kv: rename ReadWithinUncertaintyIntervalError.ExistingTimestamp to ValueTimestamp · 7a27bc59
      Nathan VanBenschoten authored
      Simple proto field rename. No compatibility concerns.
      
      Release note: None
      Epic: None
      7a27bc59
    • Andy Yang's avatar
      roachtest: update activerecord blocklist · 74835907
      Andy Yang authored
      This patch removes a few tests from the blocklist that do not
      exist in the test suite for rails 7.0.3, which were being run
      prior to the fix for version pinning.
      
      Release note: None
      74835907
    • maryliag's avatar
      ui: hide list of statement for non-admins · cf7f7bb5
      maryliag authored
      The get the list of fingerprints used by an index,
      we use the `system.statement_statistics` directly, but
      non-admins don't have access to system table.
      Until #95756 or #95770 are done, we need to hide
      this feature for non-admins.
      
      Part Of #93087
      
      Release note (ui change): Hide list of used fingerprint per index
      on Index Details page, for non-admin users.
      cf7f7bb5
    • craig[bot]'s avatar
      Merge #95631 #95636 #95691 · 6819d87c
      craig[bot] authored
      95631: schemachanger: generalized statement-time execution r=postamar a=postamar
      
      This change makes it possible to plan and execute the following change
      correctly:
      
          BEGIN;
          DROP TABLE foo;
          CREATE UNIQUE INDEX idx ON bar (x);
          COMMIT;
      
      Inside the transaction, the table is seen as dropped right after the
      DROP TABLE statement executes, but the table is not seen as dropped by
      other transactions until the new, unrelated index has been validated, at
      which point the schema change can no longer fail.
      
      Fixes https://github.com/cockroachdb/cockroach/issues/88294
      
      .
      
      Significant changes:
      - descs.Collection has a new ResetUncommitted method.
      - scstage.BuildStages is rewritten with unrolled loops and a new scheme:
        statement phase has revertible op-edges, pre-commit has two stages,
        one which resets the uncommitted state by scheduling the new
        scop.UndoAllInTxnImmediateMutationOpSideEffects op, which triggers a call
        to ResetUncommitted.
      - This happens in scexec.executeMutationOps, which is now split into two
        pieces, the first one which executes ops whose side-effects can be undone
        by ResetUncommitted, and the other which handles those which can’t.
      - These ops implement scop.ImmediateMutationOp and scop.DeferredMutationOp
        respectively.
      - These have their own visitors with their own state and their own
        dependencies.
      
      This all means that we can handle DROPs properly now: the TXN_DROPPED element
      state is removed as are all the synthetic descriptor manipulations (outside of
      index and constraint validation, that is). Furthermore, the
      scgraph.PreviousTransactionPrecedence edge kind no longer serves any purpose
      and has been replaced by PreviousStagePrecedence everywhere.
      
      There’s a bunch of other more-or-less related changes, mostly as a consequence
      to the above:
      - scbuild.Build must produce a tighter target set: no-op targets are
        elided.
      - nstree.MutableCatalog has new methods for deleting comments and zone configs
        which are useful.
      - sctestdeps.TestState has better storage layers modelling
        (in-memory > stored > committed)
      - Added missing “relation dropped before column|index no longer public” rules
        which fix DROP COLUMN CASCADE when the column is referenced in a view.
      - scgraph.Graph has a new GetOpEdgeTo method which is useful.
      - added debug info to assertion error messages in scstage.BuildStages to make
        debugging easier during development.
      - EXPLAIN (DDL) output has a nicer root node label which puts the last
        statement first.
      
      Release note: None
      
      95636: sql: increase the online help for SHOW RANGES r=ecwall a=knz
      
      The output of `\h SHOW RANGES` wasn't mentioning the KEYS option. Now it does. Also this adds some additional details about the various options.
      
      Release note: None
      Epic: None
      
      95691: sql: improve tenant records r=stevendanna,dt,ajwerner a=knz
      
      supersedes #95574, #95581, #95655
      Epic: CRDB-21836
      
      **TLDR** This commit contains the following changes:
      - rename "state" to "data_state", "active" to "ready"
      - stored, non-virtual columns for "name", "data_state", "service_mode"
      - deprecate the column "active" since it mirrors "data_state'
      - move `descpb.TenantInfo` to new package `mtinfopb`.
      - new statements `ALTER TENANT ... START SERVICE EXTERNAL/SHARED`,
        `STOP SERVICE` to change the service mode.
      
      Details follow.
      
      **rename TenantInfo.State to DataState, "ACTIVE" to "READY"**
      
      We've discovered that we'd like to separate the readiness of the data
      from the activation of the service. To emphasize this, this commit
      renames the field "State" to "DataState".
      
      Additionally, the state "ACTIVE" was confusing as it suggests that
      something is running, whereas it merely marks the tenant data as ready
      for use. So this commit also renames that state accordingly.
      
      **new tenant info field ServiceMode**
      
      Summary of changes:
      - the new TenantInfo.ServiceMode field indicates how to run servers.
      - new syntax: `ALTER TENANT ... START SERVICE EXTERNAL/SHARED`,
        `ALTER TENANT ... STOP SERVICE`.
      - tenants created via `create_tenant(<id>)`
        (via CC serverless control plane) start in service mode EXTERNAL.
      - other tenants start in service mode NONE.
      - need ALTER TENANT STOP SERVICE before dropping a tenant.
        - except in the case of `crdb_internal.destroy_tenant`
          for compat with CC serverless control plane.
      
      **make the output columns of SHOW TENANT lowercase**
      
      All the SHOW statements report status-like data in lowercase.
      SHOW TENANT(s) should not be different.
      
      **use actual SQL columns for the TenantInfo fields**
      
      Release note: None
      
      Co-authored-by: default avatarMarius Posta <marius@cockroachlabs.com>
      Co-authored-by: kena's avatarRaphael 'kena' Poss <knz@thaumogen.net>
      6819d87c
    • Nathan VanBenschoten's avatar
      storage: add CommitNoSyncWait and SyncWait to Batch interface · efe12b87
      Nathan VanBenschoten authored
      Extracted from #94165.
      
      Picks up github.com/cockroachdb/pebble/pull/2117.
      
      Release note: None
      Epic: None
      efe12b87
    • maryliag's avatar
      ui: add check for cpu usage · 63caa614
      maryliag authored
      Add a check, for cases when the value might no be
      returned (cluster with mixed versions).
      
      Part Of #87213
      
      Release note: None
      63caa614
    • kena's avatar
      sql: improve tenant records · 5fcce464
      kena authored
      This commit contains the following changes:
      - rename "state" to "data_state", "active" to "ready"
      - stored, non-virtual columns for "name", "data_state", "service_mode"
      - deprecate the column "active" since it mirrors "data_state'
      - move `descpb.TenantInfo` to new package `mtinfopb`.
      - new statements `ALTER TENANT ... START SERVICE EXTERNAL/SHARED`,
        `STOP SERVICE` to change the service mode.
      
      Details follow.
      
      **rename TenantInfo.State to DataState, "ACTIVE" to "READY"**
      
      We've discovered that we'd like to separate the readiness of the data
      from the activation of the service. To emphasize this, this commit
      renames the field "State" to "DataState".
      
      Additionally, the state "ACTIVE" was confusing as it suggests that
      something is running, whereas it merely marks the tenant data as ready
      for use. So this commit also renames that state accordingly.
      
      **new tenant info field ServiceMode**
      
      Summary of changes:
      - the new TenantInfo.ServiceMode field indicates how to run servers.
      - new ...
      5fcce464
    • kena's avatar
      sql: improve the handling of tenant capabilities · 17827af8
      kena authored
      The previous change had a few shortcomings:
      - did not properly separate prepare/planning from execute.
      - put evaluation code in the 'tree' package (we want it in sql)
      - didn't set a foundation for non-bool capabilities
      
      This commit fixes that.
      
      Release note: None
      17827af8
    • craig[bot]'s avatar
      Merge #95894 · 9caf7581
      craig[bot] authored
      
      
      95894: sql: correctly handle COLLATEd columns in COPY r=cucaroach a=otan
      
      See individual commits for details.
      
      Resolves #95887
      
      Release justification: fixes a critical bug with COPY
      
      Co-authored-by: default avatarOliver Tan <otan@cockroachlabs.com>
      9caf7581
    • Oliver Tan's avatar
      sql: fix collated strings during COPY · 5b41f6e9
      Oliver Tan authored
      Release note (bug fix): Fix a bug where COPYing into a column with
      collated strings would result in an error similar to
      `internal error: unknown type collatedstring`.
      5b41f6e9
    • craig[bot]'s avatar
      Merge #92345 #95850 · 2ad8df3d
      craig[bot] authored
      92345: changefeedccl: sqlsmith test r=[mgartner,jayshrivastava] a=HonoreDB
      
      Informs https://github.com/cockroachdb/cockroach/issues/83591
      
      
      
      Adds a test generating random predicates and
      verifies that (except for a few known issues)
      if a changefeed runs successfully it will output
      the same rows as a regular query.
      
      Release note: None
      
      95850: rules: avoid O(columns^2) behavior with a small change r=ajwerner a=ajwerner
      
      Epic: None
      
      Release note (performance improvement): Fixed a bug which could lead to very slow drop when tables or views have a very large number of columns >1000.
      
      Co-authored-by: default avatarAaron Zinger <zinger@cockroachlabs.com>
      Co-authored-by: Andrew Werner's avatarAndrew Werner <awerner32@gmail.com>
      2ad8df3d
    • Oliver Tan's avatar
      tree: rename ParseTimeContext to ParseContext · 7b52430c
      Oliver Tan authored
      A mechanical rename PR. Anything named after ParseTimeContext is now
      just ParseContext.
      
      Release note: None
      7b52430c
    • Marius Posta's avatar
      schemachanger: generalized statement-time execution · 083e17ad
      Marius Posta authored
      This change makes it possible to plan and execute the following change
      correctly:
      
          BEGIN;
          DROP TABLE foo;
          CREATE UNIQUE INDEX idx ON bar (x);
          COMMIT;
      
      Inside the transaction, the table is seen as dropped right after the
      DROP TABLE statement executes, but the table is not seen as dropped by
      other transactions until the new, unrelated index has been validated, at
      which point the schema change can no longer fail.
      
      Fixes #88294.
      
      Significant changes:
      - descs.Collection has a new ResetUncommitted method.
      - scstage.BuildStages is rewritten with unrolled loops and a new scheme:
        statement phase has revertible op-edges, pre-commit has two stages,
        one which resets the uncommitted state by scheduling the new
        scop.UndoAllInTxnImmediateMutationOpSideEffects op, which triggers a call
        to ResetUncommitted.
      - This happens in scexec.executeMutationOps, which is now split into two
        pieces, the first one which executes ops whose side-effects can be undone
        by ResetUncommitted, and the other which handles those which can’t.
      - These ops implement scop.ImmediateMutationOp and scop.DeferredMutationOp
        respectively.
      - These have their own visitors with their own state and their own
        dependencies.
      
      This all means that we can handle DROPs properly now: the TXN_DROPPED
      element state is removed as are all the synthetic descriptor manipulations
      (outside of index and constraint validation, that is). Furthermore, the
      scgraph.PreviousTransactionPrecedence edge kind no longer serves any
      purpose and has been replaced by PreviousStagePrecedence everywhere.
      
      There’s a bunch of other more-or-less related changes, mostly as a
      consequence to the above:
      - scbuild.Build must produce a tighter target set: no-op targets are
        elided.
      - nstree.MutableCatalog has new methods for deleting comments and zone
        configs which are useful.
      - sctestdeps.TestState has better storage layers modelling
        (in-memory > stored > committed)
      - Added missing “relation dropped before column|index no longer public”
        rules which fix DROP COLUMN CASCADE when the column is referenced in
        a view.
      - scgraph.Graph has a new GetOpEdgeTo method which is useful.
      - added debug info to assertion error messages in scstage.BuildStages to
        make debugging easier during development.
      - EXPLAIN (DDL) output has a nicer root node label which puts the last
        statement first.
      
      Release note: None
      083e17ad
    • craig[bot]'s avatar
      Merge #95834 #95859 · f49b07ce
      craig[bot] authored
      
      
      95834: go.mod: bump Pebble to 4199154043c5 r=nicktrav a=jbowens
      
      ```
      41991540 db: tweak to BenchmarkIteratorScanNextPrefix and performance commentary
      bb1a420f internal/rangekey: optimize Coalesce
      76159f92 sstable: remove Writer.RangeKey{Set,Unset,Delete} coalescing
      9057cd24 sstable: update two sstable benchmarks to also read the value
      012c0ce3 replay: unflake TestCollectCorpus
      152a19ea *: apply Go 1.19
      ```
      
      Epic: None
      Release note: None
      
      95859: sql: skip slow benchmark with the short flag r=ajwerner a=ajwerner
      
      Epic: none
      
      Release note: None
      
      Co-authored-by: default avatarJackson Owens <jackson@cockroachlabs.com>
      Co-authored-by: Andrew Werner's avatarAndrew Werner <awerner32@gmail.com>
      f49b07ce
    • craig[bot]'s avatar
      Merge #95897 · 19d37b81
      craig[bot] authored
      
      
      95897: logictest: document nodeidx=N option for query directives r=otan a=andyyang890
      
      Epic: None
      
      Release note: None
      
      Co-authored-by: default avatarAndy Yang <yang@cockroachlabs.com>
      19d37b81
    • Andy Yang's avatar
      logictest: document nodeidx=N option for query directives · 5e2ad8ec
      Andy Yang authored
      Release note: None
      5e2ad8ec
  3. 25 Jan, 2023 6 commits
    • Andrew Werner's avatar
      sql: avoid checking if a role exists when setting to the current role · 74487fc8
      Andrew Werner authored
      This is an uncached round-trip. It makes `DISCARD` expensive.
      In https://github.com/cockroachdb/cockroach/pull/86485 we implemented
      `SET SESSION AUTHORIZATION DEFAULT`, this added an extra sql query to
      `DISCARD ALL` to check if the role we'd become exists. This is almost
      certainly unintentional in the case where the role we'd become is the
      current session role. I think this just happened because of code
      consolidation.
      
      We now no longer check if the current session role exists. This removes
      the last round-trip from DISCARD ALL.
      
      Release note (performance improvement): In 22.2, logic was added to make
      `SET SESSION AUTHORIZATION DEFAULT` not a no-op. This implementation used
      more general code for setting the role for a session which made sure that
      the role exists. The check for whether a role exists is currently uncached.
      We don't need to check if the role we already are exists. This improves the
      performance of `DISCARD ALL` in addition to `SET SESSION AUTHORIZATION
      DEFAULT`.
      74487fc8
    • Andrew Werner's avatar
      sql: use in-memory session data to decide what to do in DISCARD · 73befd93
      Andrew Werner authored
      In #86246 we introduced logic to discard schemas when running DISCARD ALL and
      DISCARD TEMP. This logic did expensive kv operations unconditionally; if the
      session knew it had never created a temporary schema, we'd still fetch all
      databases and proceed to search all databases for a temp schema. This is very
      expensive.
      
      Fixes: #95864
      
      Release note (performance improvement): In 22.2, we introduced support for
      `DISCARD TEMP` and made `DISCARD ALL` actually discard temp tables. This
      implementation ran expensive logic to discover temp schemas rather than
      consulting in-memory data structures. As a result, `DISCARD ALL`, which
      is issued regularly by connection pools, became an expensive operation
      when it should be cheap. This problem is now resolved.
      73befd93
    • craig[bot]'s avatar
      Merge #95013 #95797 #95830 #95852 #95863 · 48580950
      craig[bot] authored
      95013: sql: add ability set, edit, read tenant capabilities r=knz a=ecwall
      
      Fixes #87851
      
      Add new SQL syntax for
      1) Setting tenant capabilities:
      `ALTER TENANT t GRANT CAPABILITY capabilitiy_name=capability_value;`
      2) Resetting tenant capabilities:
      `ALTER TENANT t REVOKE CAPABILITIY capability_name;`
      3) Reading tenant capabilities:
      `SHOW TENANT t WITH CAPABILITIES;`
      
      Release note: None
      
      95797: sql: improve stack trace for get-user-timeout timeouts r=knz a=ecwall
      
      Fixes #95794
      
      The cause of the `get-user-timeout` errors is unknown. Part of the problem is that the stack trace gets cut off at
      ```
        |   | github.com/cockroachdb/cockroach/pkg/sql.retrieveSessionInitInfoWithCache
        |   | 	github.com/cockroachdb/cockroach/pkg/sql/user.go:238
      ```
      which does not explain what is actually being blocked.
      
      The reason that the stack trace is cut off is that the timeout is initiated by `contextutil.RunWithTimeout` which results in a "simple" (no stack trace) `context.DeadlineExceeded` error.
      
      `retrieveSessionInitInfoWithCache` is the first line in the stack trace because it calls `errors.Wrap` on `context.DeadlineExceeded`.
      
      To get a fuller stack trace, `context.DeadlineExceeded` must be wrapped immediately (`errors.Wrap` or `errors.WithStack`) before it bubbles up.
      
      Release note: None
      
      95830: validate: use immutable descriptors only r=postamar a=postamar
      
      The descriptor validation logic will accept any implementation of catalog.Descriptor be it mutable or immutable, it doesn't care. However, using mutable implementations can have a significant performance impact especially in the case of tables, where every column or index or constraint lookup will lead to the cache being regenerated for the whole descriptor.
      
      This commit fixes this by having validate.Validate replace any mutable descriptor instances it encounters with immutable copies. This doesn't change anything except performance.
      
      Fixes #95827.
      
      Release note: None
      
      95852: ui: cache sqlroles results r=maryliag a=maryliag
      
      Previously, the call to get sql roles was constantly being requested. This commits adds a cache limit, so it will only get request after the expiration time.
      
      https://www.loom.com/share/6814309f91234fa2b17490df8160bde6
      
      
      
      Epic: None
      Release note: None
      
      95863: storage: reorder EventListeners r=jbowens a=jbowens
      
      To be defensive, sequence the EventListener responsible for crashing the process during a disk stall first, before the Pebble logging event listener.
      
      Informs #94373.
      Epic: None
      Release note: None
      
      Co-authored-by: default avatarEvan Wall <wall@cockroachlabs.com>
      Co-authored-by: default avatarMarius Posta <marius@cockroachlabs.com>
      Co-authored-by: default avatarmaryliag <marylia@cockroachlabs.com>
      Co-authored-by: default avatarJackson Owens <jackson@cockroachlabs.com>
      48580950
    • Andrew Werner's avatar
      rttanalysis: add a DISCARD benchmark · 66e265ed
      Andrew Werner authored
      Epic: none
      
      Release note: None
      66e265ed
    • Aaron Zinger's avatar
      changefeedccl: allow random expressions referencing boolean columns · 469222fa
      Aaron Zinger authored
      Removes the special case in TestChangefeedRandomExpressions for boolean
      columns as these are now supported.
      
      Release note: None
      469222fa
    • Xin Hao Zhang's avatar
      insights: record insights when txn contention meets threshold · de5fbabd
      Xin Hao Zhang authored
      Part of #90393
      
      Previously, we only write a transaction and its statements
      to the insights system when there is an issue detected for
      one of the statements in the transaction. However, a transaction
      should still report high contention when the total amount of
      contention time is high, even if the contention experienced
      by each of its statements is not over the threshold.
      
      This commit ensures that transactions which have a recorded
      contention time above the insights latency threshold get
      reported to the insights system.
      
      Release note: None
      de5fbabd