sql: report unit for some session variables

[Guilherme1Rocha]

Previously, we never populated the unit column of pg_settings
virtual table deviating from PostgreSQL. We now report the unit for
time-based and memory-based settings.

AFAIU the unit defines the default unit of the value which allows these
settings take in integer numbers with the unit providing the necessary
measurement dimension.

Fixes: #30717
Release note: None

[blathers-crl]

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

Before a member of our team reviews your PR, I have some potential action items for you:

• Please ensure your git commit message contains a release note.
• When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[cockroach-teamcity]

This change is 

[cockroachlabs-cla-agent]

All committers have signed the CLA.

[Guilherme1Rocha]

@yuzefovich, sorry to bother you, would you mind taking a look at this PR?

I've added a map for time-related units, allowing developers to customize which unit to use. However, since PostgreSQL often normalizes time-based values to milliseconds I'm not sure how useful this mapping actually is.

If you see anything that could be improved or adjusted, please let me know. Thanks!

[yuzefovich]

Thanks for looking into this!

This seems reasonable to me, but I do have two high-level suggestions that should improve / simplify the change:

• first, rather than including another return parameter into sessionVar.Get function, I think it'd be cleaner if we introduced another function GetUnit func(evalCtx *extendeEvalContext) string that would return the unit. This field can remain unset for vast majority of session variables, and the caller of sessionVar.GetUnit will be required to check that the function is non-nil. (Sorry, you had to do all the plumbing with the current approach - I hope you were able to automate it using AI tools so that not a of time / effort was spent on that.)
• for testing, I'd be a bit cleaner to use the logic test framework. In fact, we already have a query in pkg/sql/logictest/testdata/logic_test/pg_catalog that shows the unit. I think that should be sufficient - you'll need to do something like ./dev testlogic base --files=pg_catalog --config=local --rewrite -v to update the expected output given the new behavior.

Please let me know if this makes sense and / or more details would be helpful.

Reviewed 9 of 9 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Guilherme1Rocha)

[blathers-crl]

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

• We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
• Please ensure your git commit message contains a release note.
• When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[blathers-crl]

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

• We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
• Please ensure your git commit message contains a release note.
• When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[blathers-crl]

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

• We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
• Please ensure your git commit message contains a release note.
• When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[yuzefovich]

Thanks for the update! I cleaned up a couple of nits and pushed the update to kick off the CI.

I still need to review this change more carefully since I find the behavior of PG somewhat non-intuitive, but there is no action needed on your part (at least for now). For example, this is a bit confusing:

yuzefovich=# select name,setting,unit from pg_settings where name = 'work_mem';
   name   | setting | unit 
----------+---------+------
 work_mem | 4096    | kB
(1 row)

yuzefovich=# show work_mem;
 work_mem 
----------
 4MB
(1 row)

Reviewed 6 of 12 files at r2, 6 of 6 files at r3, 10 of 10 files at r4, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Guilherme1Rocha)

[yuzefovich]

I refactored the change a bit to be closer to behavior of PG. @Guilherme1Rocha thanks for your contribution!

Reviewed 6 of 8 files at r5, 3 of 3 files at r6, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @Guilherme1Rocha)

[yuzefovich]

@DrewKimball could you please take a look at this with fresh eyes?

[yuzefovich]

Friendly ping @DrewKimball (or maybe someone else from @cockroachdb/sql-queries-prs).

[DrewKimball]

Sorry about that, missed the initial ping. I'll take a look.

[DrewKimball]

Nice!

Reviewed 7 of 10 files at r4, 5 of 8 files at r5, 3 of 3 files at r6, all commit messages.
Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @Guilherme1Rocha)

[yuzefovich]

Thanks for taking a look!

bors r=yuzefovich,drewkimball

[craig]

Build succeeded:

• examples_orms
• docker_image_amd64
• linux_amd64_fips_build
• local_roachtest_fips
• acceptance
• linux_amd64_build
• unit_tests
• check_generated_code
• lint
• local_roachtest