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: update grant_table logictests to not cause massive diffs #140278

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

normanchenn
Copy link
Member

Previously, there was no ordering in the grant_table logictest queries, causing any rewrite of these logic tests (i.e. the result of adding types) to result in large diffs. Now, these queries are ordered so the diffs will make more sense.

Epic: None
Release note: None

Copy link

blathers-crl bot commented Jan 31, 2025

Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks.

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

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@normanchenn normanchenn force-pushed the norman/order-grant_table branch from f1c32cc to f12d1ca Compare February 3, 2025 15:17
Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up! I have just one nit.

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


pkg/sql/logictest/testdata/logic_test/grant_table line 24 at r1 (raw file):

# to filter the built-in internal vtables down to just one so that we are not
# enumerating all builtins and being overly broad and brittle.
query TTTTTTB colnames,rowsort

nit: let's remove rowsort directives where we now have proper ORDER BY clauses.

Previously, there was no ordering in the grant_table logictest queries,
causing any rewrite of these logic tests (i.e. the result of adding types)
to result in large diffs. Now, these queries are ordered so the diffs
will make more sense.

Epic: None
Release note: None
@normanchenn normanchenn force-pushed the norman/order-grant_table branch from f12d1ca to a6b6d45 Compare February 3, 2025 19:46
@normanchenn normanchenn marked this pull request as ready for review February 3, 2025 20:19
@normanchenn normanchenn self-assigned this Feb 3, 2025
@cockroachdb cockroachdb deleted a comment from craig bot Feb 3, 2025
Copy link
Member

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

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

bors r+

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @normanchenn)

@craig
Copy link
Contributor

craig bot commented Feb 3, 2025

Build failed (retrying...):

@craig craig bot merged commit 95d43db into cockroachdb:master Feb 3, 2025
22 checks passed
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