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: support backslash_quote session setting #45774

Open
florence-crl opened this issue Mar 5, 2020 · 4 comments
Open

sql: support backslash_quote session setting #45774

florence-crl opened this issue Mar 5, 2020 · 4 comments
Labels
E-starter Might be suitable for a starter project for new employees or team members. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@florence-crl
Copy link

florence-crl commented Mar 5, 2020

Is your feature request related to a problem? Please describe.
A customer is trying to use CRDB with Sonarqube. Sonarqube tries to read or set a PostgreSQL configuration setting called backslash_quote and is getting this error:

Caused by: java.sql.SQLException: Cannot create PoolableConnectionFactory (ERROR: unimplemented: the configuration setting "backslash_quote" is not supported
  Hint: You have attempted to use a feature that is not yet implemented.

Describe the solution you'd like
Customer asks:
(1) Can reading/setting this configuration setting be implemented in CRDB?
(2) What is the size of this change?

Describe alternatives you've considered
If it's something that might take time, they can use another solution while a fix is developed, then migrate to CockroachDB after the fix.

Additional context
from @rafiss
we could also throw in a CockroachDB dialect into sonarqube pretty easily. here's the PG one.

Jira issue: CRDB-5129

@jordanlewis jordanlewis changed the title Enable CRDB compatibility with Sonarqube by implementing Postgresql configuration setting backslash_quote sql: support backslash_quote session setting Mar 5, 2020
@RoachietheSupportRoach
Copy link
Collaborator

Zendesk ticket #4832 has been linked to this issue.

@rafiss
Copy link
Collaborator

rafiss commented Mar 5, 2020

Here is a PR for adding CockroachDB support within SonarQube: SonarSource/sonarqube#3237

@florence-crl
Copy link
Author

florence-crl commented Mar 6, 2020

Sonarqube decided that CockroachDB is not eligible for official support for now, so please implement this setting on the CRDB side.

The customer is currently using v19.2.4 in cockroachCloud and 7.9.2-community for Sonarqube.

@rafiss rafiss assigned rafiss and apantel and unassigned rafiss Mar 31, 2020
@rafiss rafiss added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label May 12, 2021
@rafiss rafiss added the E-starter Might be suitable for a starter project for new employees or team members. label Jun 21, 2021
ZhouXing19 added a commit to ZhouXing19/cockroach that referenced this issue Jul 7, 2021
This setting is based on an previous issue: cockroachdb#45774. This commit set a placeholder for PostgreSQL configuration setting called (backslash_quote)[https://www.postgresql.org/docs/12/runtime-config-compatible.html#GUC-BACKSLASH-QUOTE], and ensured it is compatible with ORMS.

When users try setting the backslash_quote session variable, CRDB will let it pass, but not making any changes. We leave the functionality implementation to future works.

Details:
- Added `backslash_quote` at varGen at `sql/vars.go` without implementing the true functionality

- Tested it with `subtest backslash_quote_test` at `pkg/sql/logictest/testdata/logic_test/set`

- Removed `backslash_quote` from `unsupported_vars.go`
ZhouXing19 added a commit to ZhouXing19/cockroach that referenced this issue Jul 8, 2021
This setting is based on an previous issue: cockroachdb#45774. This commit set a placeholder for PostgreSQL configuration setting called (backslash_quote)[https://www.postgresql.org/docs/12/runtime-config-compatible.html#GUC-BACKSLASH-QUOTE], and ensured it is compatible with ORMS.

When users try setting the backslash_quote session variable, CRDB will let it pass, but not making any changes. We leave the functionality implementation to future works.

Details:
- Added `backslash_quote` at varGen at `sql/vars.go` without implementing the true functionality

- Tested it with `subtest backslash_quote_test` at `pkg/sql/logictest/testdata/logic_test/set`

- Removed `backslash_quote` from `unsupported_vars.go`

Updates:

- nil: updated url for backslash_quote, and removed extra spaces

- Deleted `backslash_quote` from `unsupported_vars.go`

- Added `./.idea` at `.gitignore`

- Ran `make testlogic FILES="variables show_source pg_catalog" TESTFLAGS=-rewrite` but with the error: `E210708 03:58:33.948439 1281 sql/conn_executor.go:950  [n1,client=127.0.0.1:64106,hostssl,user=root] 1  error deleting temporary objects at session close, the temp tables deletion job will retry periodically: failed to send RPC: sending to all replicas failed; last error: node unavailable; try another peer`
ZhouXing19 added a commit to ZhouXing19/cockroach that referenced this issue Jul 8, 2021
This setting is based on an previous issue: cockroachdb#45774. This commit set a placeholder for PostgreSQL configuration setting called (backslash_quote)[https://www.postgresql.org/docs/13/runtime-config-compatible.html#GUC-BACKSLASH-QUOTE], and ensured it is compatible with ORMS.

When users try setting the backslash_quote session variable, CRDB will let it pass, but not making any changes. We leave the functionality implementation to future works.

Details:
- Added `backslash_quote` at varGen at `sql/vars.go` without implementing the true functionality

- Tested it with `subtest backslash_quote_test` at `pkg/sql/logictest/testdata/logic_test/set`

- Removed `backslash_quote` from `unsupported_vars.go`

Updates:

- nil: updated url for backslash_quote, and removed extra spaces

- Deleted `backslash_quote` from `unsupported_vars.go`

- Added `./.idea` at `.gitignore`

- Ran `make testlogic FILES=information_schema TESTFLAGS=-rewrite`

Release note (sql change): Added session variable backslash_quote for compatibility, setting it does a no-op.
ZhouXing19 added a commit to ZhouXing19/cockroach that referenced this issue Jul 9, 2021
This setting is based on an previous issue: cockroachdb#45774. This commit set a placeholder for PostgreSQL configuration setting called (backslash_quote)[https://www.postgresql.org/docs/13/runtime-config-compatible.html#GUC-BACKSLASH-QUOTE], and ensured it is compatible with ORMS. Currently, we only allow the value to be `backslash_quote`.  If `client_encoding` is updated to allow encodings other than UTF8, then the default value of `backslash_quote` should be changed to `on`.

When users try setting the backslash_quote session variable, CRDB will let it pass, but not making any changes. We leave the functionality implementation to future works.

Details:
- Added `backslash_quote` at varGen at `sql/vars.go` without implementing the true functionality

- Tested it with `subtest backslash_quote_test` at `pkg/sql/logictest/testdata/logic_test/set`

- Removed `backslash_quote` from `unsupported_vars.go`

Updates:

- nil: updated url for backslash_quote, and removed extra spaces

- Deleted `backslash_quote` from `unsupported_vars.go`

- Added `./.idea` at `.gitignore`

- Ran `make testlogic FILES=information_schema TESTFLAGS=-rewrite`

Release note (sql change): Added session variable backslash_quote for compatibility, setting it does a no-op, and only safe_encoding is supported.
ZhouXing19 added a commit to ZhouXing19/cockroach that referenced this issue Jul 9, 2021
This setting is based on an previous issue: cockroachdb#45774. This commit set a placeholder for PostgreSQL configuration setting called (backslash_quote)[https://www.postgresql.org/docs/13/runtime-config-compatible.html#GUC-BACKSLASH-QUOTE], and ensured it is compatible with ORMS. Currently, we only allow the value to be `backslash_quote`.  If `client_encoding` is updated to allow encodings other than UTF8, then the default value of `backslash_quote` should be changed to `on`.

When users try setting the backslash_quote session variable, CRDB will let it pass, but not making any changes. We leave the functionality implementation to future works.

Details:
- Added `backslash_quote` at varGen at `sql/vars.go` without implementing the true functionality

- Tested it with `subtest backslash_quote_test` at `pkg/sql/logictest/testdata/logic_test/set`

- Removed `backslash_quote` from `unsupported_vars.go`

Updates:

- nil: updated url for backslash_quote, and removed extra spaces

- Deleted `backslash_quote` from `unsupported_vars.go`

- Added `./.idea` at `.gitignore`

- Ran `make testlogic FILES=information_schema TESTFLAGS=-rewrite`

Release note (sql change): Added session variable backslash_quote for compatibility, setting it does a no-op, and only safe_encoding is supported.
craig bot pushed a commit that referenced this issue Jul 9, 2021
67291: execgen: preserve comments around callsite of inlined function r=DrewKimball a=DrewKimball

This patch modifies the execgen `inline` directive to ensure that
comments before and after the callsites of inlined functions are
not removed. Example:
```
func a() {
  // This comment should not be removed.
  b()
  // This one too.
}

// execgen:inline
func b() {
  foo = bar
}
=>
func a() {
  // This comment should not be removed.
  {
    foo = bar
  }
  // This one too.
}

// execgen:inline
const _ = "inlined_b"
```

Release note: None

67343: sql: support backslash_quote session setting r=ZhouXing19 a=ZhouXing19

This setting is based on a previous issue: #45774. This commit set a placeholder for PostgreSQL configuration setting called [backslash_quote](https://www.postgresql.org/docs/12/runtime-config-compatible.html#GUC-BACKSLASH-QUOTE), and ensured it is compatible with ORMS. Currently, we only allow the value to be `backslash_quote`.  If `client_encoding` is updated to allow encodings other than UTF8, then the default value of `backslash_quote` should be changed to `on`.

When users try setting the backslash_quote session variable, CRDB will let it pass, but not making any changes. We leave the functionality implementation to future works.

Details:
- Added `backslash_quote` at varGen at `sql/vars.go` without implementing the true functionality

- Tested it with `subtest backslash_quote_test` at `pkg/sql/logictest/testdata/logic_test/set`

- Removed `backslash_quote` from `unsupported_vars.go`

Passed local test:
<img src="https://i.imgur.com/crZFk9w.png">

Updates:

- nil: updated url for backslash_quote, and removed extra spaces

- Deleted `backslash_quote` from `unsupported_vars.go`

- Ran `make testlogic FILES=information_schema TESTFLAGS=-rewrite`

Release note (sql change): Added session variable backslash_quote for compatibility, setting it does a no-op, and only `safe_encoding` is supported.


67399: execgen: fix template function names with argument comments r=mgartner a=mgartner

#### execgen: do not silently err when producing output string

Previously, execgen would silently fail if `decorator.Fprint` returned
an error. This could happen if templating logic created an invalid DST.
The result of the silent failure was an empty `.eg.go` file without any
indication of what went wrong. Errors are now more explicit which should
give us a starting place for tracking down templating bugs.

Release note: None

#### execgen: fix template function names with argument comments

Previously, execgen would generate invalid DSTs for template function
variants when call sites had comments describing an argument. For
example:

    func a() {
        b(true /* t */)
    }

    // execgen:inline
    // execgen:template<t>
    func b(t bool) {
        // ...
    }

This would produce a template function variant with `/* t */`, which was
invalid because it contains spaces.

Template argument decorations are now cleared to avoid this issue.

Release note: None


Co-authored-by: Drew Kimball <[email protected]>
Co-authored-by: Jane Xing <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
@github-actions
Copy link

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 26, 2023
@yuzefovich yuzefovich reopened this May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-starter Might be suitable for a starter project for new employees or team members. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

No branches or pull requests

5 participants