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: implement format() builtin #68330

Closed
RemiKalbe opened this issue Aug 2, 2021 · 3 comments · Fixed by #84107
Closed

sql: implement format() builtin #68330

RemiKalbe opened this issue Aug 2, 2021 · 3 comments · Fixed by #84107
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) X-blathers-triaged blathers was able to find an owner

Comments

@RemiKalbe
Copy link

RemiKalbe commented Aug 2, 2021

Describe the problem

Using the format function in any query doesn't work.

To Reproduce

Use format()

Expected behavior
To work.

Additional data / screenshots
https://www.postgresql.org/docs/9.3/functions-string.html#FUNCTIONS-STRING-FORMAT

Environment:

  • CockroachDB version 21.1
  • Server OS: macOS 11.4

Workaround

I'm using the replace function, but using it with more than 1 argument becomes tedious.

Jira issue: CRDB-8990

@RemiKalbe RemiKalbe added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Aug 2, 2021
@blathers-crl
Copy link

blathers-crl bot commented Aug 2, 2021

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I have CC'd a few people who may be able to assist you:

  • @cockroachdb/sql-experience (found keywords: ORM)

If we have not gotten back to your issue within a few business days, you can try the following:

  • Join our community slack channel and ask on #cockroachdb.
  • Try find someone from here if you know they worked closely on the area and CC them.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added O-community Originated from the community X-blathers-triaged blathers was able to find an owner labels Aug 2, 2021
@rafiss rafiss changed the title unknown function: format() sql: implement format() builtin Aug 2, 2021
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Aug 2, 2021
@rafiss
Copy link
Collaborator

rafiss commented Aug 2, 2021

Thanks for filing this! We'll see if we can add this in a future release.

Out of curiosity, is it even more tedious to do this formatting inside of your application?

@RemiKalbe
Copy link
Author

Formating it in my app would require sanitation as I'm formatting user input; doing it this way let the SQL library I use handle it for me.

HonoreDB added a commit to HonoreDB/cockroach that referenced this issue Jul 12, 2022
Closes cockroachdb#68330.

Unfortunately hooking into fmt by implementing the Formatter interface
doesn't give us the postgres-style syntax, so instead I've
implemented my own very stripped-down version of fmt by
copying golang's.

Release note (sql change): Added the format builtin function. format interpolates arguments into a string in the style of C's sprintf. For example, format('Hello, %s', 'world') returns 'Hello, world'.
HonoreDB added a commit to HonoreDB/cockroach that referenced this issue Jul 14, 2022
Closes cockroachdb#68330.

Unfortunately hooking into fmt by implementing the Formatter interface
doesn't give us the postgres-style syntax, so instead I've
implemented my own very stripped-down version of fmt by
copying golang's.

Release note (sql change): Added the format builtin function.
format interpolates arguments into a string in the style of C's sprintf.
For example, format('Hello, %s', 'world') returns 'Hello, world'.
HonoreDB added a commit to HonoreDB/cockroach that referenced this issue Jul 14, 2022
Closes cockroachdb#68330.

Unfortunately hooking into fmt by implementing the Formatter interface
doesn't give us the postgres-style syntax, so instead I've
implemented my own very stripped-down version of fmt by
copying golang's.

Release note (sql change): Added the format builtin function.
format interpolates arguments into a string in the style of C's sprintf.
For example, format('Hello, %s', 'world') returns 'Hello, world'.
HonoreDB added a commit to HonoreDB/cockroach that referenced this issue Jul 16, 2022
Closes cockroachdb#68330.

Unfortunately hooking into fmt by implementing the Formatter interface
doesn't give us the postgres-style syntax, so instead I've
implemented my own very stripped-down version of fmt by
copying golang's.

Release note (sql change): Added the format builtin function.
format interpolates arguments into a string in the style of C's sprintf.
For example, format('Hello, %s', 'world') returns 'Hello, world'.
HonoreDB added a commit to HonoreDB/cockroach that referenced this issue Jul 29, 2022
Closes cockroachdb#68330.

Unfortunately hooking into fmt by implementing the Formatter interface
doesn't give us the postgres-style syntax, so instead I've
implemented my own very stripped-down version of fmt by
copying golang's.

Release note (sql change): Added the format builtin function.
format interpolates arguments into a string in the style of C's sprintf.
For example, format('Hello, %s', 'world') returns 'Hello, world'.
HonoreDB added a commit to HonoreDB/cockroach that referenced this issue Aug 4, 2022
Closes cockroachdb#68330.

Unfortunately hooking into fmt by implementing the Formatter interface
doesn't give us the postgres-style syntax, so instead I've
implemented my own very stripped-down version of fmt by
copying golang's.

Release note (sql change): Added the format builtin function.
format interpolates arguments into a string in the style of C's sprintf.
For example, format('Hello, %s', 'world') returns 'Hello, world'.
craig bot pushed a commit that referenced this issue Aug 11, 2022
84107: sql, builtins: implement format r=[rafiss] a=HonoreDB

Closes #68330.

Unfortunately hooking into fmt by implementing the Formatter interface
doesn't give us the postgres-style syntax, so instead I've
implemented my own very stripped-down version of fmt by
copying golang's.

Release note (sql change): Added the format builtin function. format interpolates arguments into a string in the style of C's sprintf. For example, format('Hello, %s', 'world') returns 'Hello, world'.

85580: storage: remove key comparison in MVCC stats computations r=jbowens a=erikgrinaker

This patch restructures MVCC stats computations to use an MVCC iterator
with appropriate bounds. This allows omitting a key comparison in the
hot path, yielding a ~10% performance improvement. The new stats
functions are:

* `ComputeStats`: calculates stats for a `Reader` key span.

* `ComputeStatsWithVisitors`: calculates stats for a `Reader` key span,
  calling the given visitor callbacks for every point/range key.

* `ComputeStatsForIter`: calculates stats for a given `MVCCIterator`, by
  scanning it until exhausted.

It also removes the `MVCCIterator.ComputeStats` method, which had no
business being part of the interface.

```
name                                     old time/op   new time/op   delta
MVCCComputeStats_Pebble/valueSize=64-24    130ms ± 1%    119ms ± 1%  -8.77%  (p=0.000 n=10+10)

name                                     old speed     new speed     delta
MVCCComputeStats_Pebble/valueSize=64-24  516MB/s ± 1%  565MB/s ± 1%  +9.61%  (p=0.000 n=10+10)
```

Resolves #41899.
Touches #84544.

Release note: None

85778: sql/catalog/tabledesc: fixed a bug in `maybeAddConstraintIDs` r=Xiang-Gu a=Xiang-Gu

Previously, when we RunPostDeserializationChanges, one of the step is
to fill in constraint IDs if they are not already set, which is done
in function `maybeAddConstraintIDs`. That function however is buggy
in that, if there is constraint mutation on the table descriptor, we
directly go assign a constraint ID (from `tbl.nextConstraintID`) to it,
without checking whether it already has a non-zero constraint ID or not.
This PR fixes that.

Release note: None

85943: sql: don't create idx recommendations for internal queries r=maryliag a=maryliag

Previously we were generating recommendations for internal
queries. This commit adds a check and if is internal, it
won't generate a recommendation.

Partially addresses #85934

Release note: None

85951: kvserver: replace ad-hoc stats in RaftTransport by metrics r=tbg a=pavelkalinnikov

These stats were only printed in text to the logs, and hard to use and match
with other existing metrics in monitoring.

There is a queue size metric introduced in 5d51ab3 which properly exports the
`queue` counter to monitoring. The exported graphs allow observing both the
current, and historical queue sizes (including the approx. max size), so this
PR safely removes both stats as obsolete.

This PR also similarly replaces all the remaining ad-hoc counters with proper
metrics, and removes the plumbing and data structures that were previously
maintaining those stats.

The granularity of the new metrics is per-node/transport. This is due to the
metrics infrastructure not being particularly good at handling high cardinality
combinations of labels such as O(N^2) node ID pairs.

Part of #83917

Release note: None

Co-authored-by: Aaron Zinger <[email protected]>
Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: Xiang Gu <[email protected]>
Co-authored-by: Marylia Gutierrez <[email protected]>
Co-authored-by: Pavel Kalinnikov <[email protected]>
@craig craig bot closed this as completed in b33b9db Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) X-blathers-triaged blathers was able to find an owner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants