Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: add support for extra session vars for pg compatibility #35116

Merged
merged 1 commit into from
Feb 25, 2019

Conversation

knz
Copy link
Contributor

@knz knz commented Feb 21, 2019

Fixes #35109.

This adds compatibility support for the following variables, with only
the default values that make sense in CockroachDB:

  • row_security
  • synchronize_seqscans,
  • lock_timeout,
  • idle_in_transaction_session_timeout

Release note: None

@knz knz requested review from jordanlewis, bobvawter and a team February 21, 2019 14:15
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz knz requested a review from a team as a code owner February 25, 2019 15:43
@knz knz requested review from maddyblue and removed request for a team February 25, 2019 15:43
@knz
Copy link
Contributor Author

knz commented Feb 25, 2019

ping

Copy link
Member

@jordanlewis jordanlewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong. You seem to be adding session variables

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @bobvawter, @jordanlewis, @knz, and @mjibson)


pkg/sql/logictest/testdata/planner_test/explain, line 203 at r1 (raw file):


query TTT
EXPLAIN SHOW TIME ZONE

Let's delete these explain tests. They are not testing anything anymore, and produce irritating diffs every time a variable is changed.

@knz
Copy link
Contributor Author

knz commented Feb 25, 2019

This seems wrong. You seem to be adding session variables

More words please? What's wrong about it?

@jordanlewis
Copy link
Member

Oops! I'm very sorry - that was a draft comment that I meant to delete. I thought that these were client config variables that weren't exposed as session variables, but then I tested it and discovered otherwise.

@knz
Copy link
Contributor Author

knz commented Feb 25, 2019

Oh thanks for clarifying. np

This adds compatibility support for the following variables, with only
the default values that make sense in CockroachDB:

- `row_security`
- `synchronize_seqscans`,
- `lock_timeout`,
- `idle_in_transaction_session_timeout`

Release note: None
Copy link
Contributor Author

@knz knz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @bobvawter, @jordanlewis, and @mjibson)


pkg/sql/logictest/testdata/planner_test/explain, line 203 at r1 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

Let's delete these explain tests. They are not testing anything anymore, and produce irritating diffs every time a variable is changed.

Done. good idea!

@knz
Copy link
Contributor Author

knz commented Feb 25, 2019

TFYR

bors r+

@craig
Copy link
Contributor

craig bot commented Feb 25, 2019

Build failed (retrying...)

@craig
Copy link
Contributor

craig bot commented Feb 25, 2019

Build failed (retrying...)

@craig
Copy link
Contributor

craig bot commented Feb 25, 2019

Build failed (retrying...)

@knz
Copy link
Contributor Author

knz commented Feb 25, 2019

bors r+

@craig
Copy link
Contributor

craig bot commented Feb 25, 2019

Not awaiting review

@knz
Copy link
Contributor Author

knz commented Feb 25, 2019

bors r-

@craig
Copy link
Contributor

craig bot commented Feb 25, 2019

Canceled

@knz
Copy link
Contributor Author

knz commented Feb 25, 2019

bors r+

@craig
Copy link
Contributor

craig bot commented Feb 25, 2019

Build failed (retrying...)

@knz
Copy link
Contributor Author

knz commented Feb 25, 2019

bors r+

@craig
Copy link
Contributor

craig bot commented Feb 25, 2019

Not awaiting review

@knz
Copy link
Contributor Author

knz commented Feb 25, 2019

bors r-

@craig
Copy link
Contributor

craig bot commented Feb 25, 2019

Canceled

@knz
Copy link
Contributor Author

knz commented Feb 25, 2019

bors r+

craig bot pushed a commit that referenced this pull request Feb 25, 2019
35077: workload: add indexes workload r=nvanbenschoten a=nvanbenschoten

Closes #34912.

This adds a new workload called `indexes`. Unlike `kv`, the workload is
specifically designed to stress secondary indexes. Its schema looks
like:

```
CREATE TABLE indexes (
    key     UUID  NOT NULL PRIMARY KEY,
    col0    INT   NOT NULL,
    col1    INT   NOT NULL,
    col2    INT   NOT NULL,
    col3    INT   NOT NULL,
    col4    INT   NOT NULL,
    col5    INT   NOT NULL,
    col6    INT   NOT NULL,
    col7    INT   NOT NULL,
    col8    INT   NOT NULL,
    col9    INT   NOT NULL,
    payload BYTES NOT NULL
)
```

and it includes knobs to specify how many of the "col" columns should be
indexed and whether they should be indexed using a UNIQUE specifier. The
workload also includes a knob to specify the size of the `payload`
column.

Release note: None

35116: sql: add support for extra session vars for pg compatibility r=knz a=knz

Fixes #35109.

This adds compatibility support for the following variables, with only
the default values that make sense in CockroachDB:

- `row_security`
- `synchronize_seqscans`,
- `lock_timeout`,
- `idle_in_transaction_session_timeout`

Release note: None

35134: storageccl: leave params in workload URIs r=dt a=dt

workload URIs do not contain anything sensitive and including them in the job make it easier see what it was actually doing.

Release note: none.

35151: sql: tolerate non-existent databases for plan cache invalidation r=knz a=knz

Fixes  #35145.

Release note (bug fix): CockroachDB again properly reports when a
database used during PREPARE does not exist any more when EXECUTE is
used.

35162: storage: remove TODOs in cmd_push_txn r=nvanbenschoten a=nvanbenschoten

This was discussed in #33523 (review) but I missed digging into it at the time. I still don't think I have my head fully wrapped around it, but I do enough to know that it's not something I want to start pulling on right now.

Release note: None

Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
Co-authored-by: David Taylor <tinystatemachine@gmail.com>
@craig
Copy link
Contributor

craig bot commented Feb 25, 2019

Build succeeded

@craig craig bot merged commit 532b684 into cockroachdb:master Feb 25, 2019
@knz knz deleted the 20190221-compat-vars branch February 25, 2019 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants