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: refactor system.namespace calls to use indexes where available #44819

Merged
merged 1 commit into from
Feb 10, 2020

Conversation

otan
Copy link
Contributor

@otan otan commented Feb 6, 2020

Resolves #44230.

Refactoring all system.namespace calls such that they always use indexes
when querying data if it previously used indexes before.

I left crdb_internal.lookup_namespace_id as functional as it was
before in that it does not lookup anything that isn't a public schema
or database. This is because the admin UI only looks up tables into the
public schema.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@otan otan force-pushed the namespace_queries branch 2 times, most recently from 7e065fa to 3e2c89f Compare February 6, 2020 17:02
Refactoring all system.namespace calls such that they always use indexes
when querying data if it previously used indexes before.

I left `crdb_internal.lookup_namespace_id` as functional as it was
before in that it does not lookup anything that isn't a public schema
or database. This is because the admin UI only looks up tables into the
public schema.

Release note: None
@otan otan force-pushed the namespace_queries branch from 3e2c89f to 9f884c4 Compare February 6, 2020 17:15
@otan otan requested review from solongordon and a team February 6, 2020 18:07
Copy link
Contributor

@solongordon solongordon left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 8 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @solongordon)

@otan
Copy link
Contributor Author

otan commented Feb 10, 2020

bors r=solongordon

craig bot pushed a commit that referenced this pull request Feb 10, 2020
44812: tree: modify TimestampTZ operators to use ctx loc for operations r=solongordon a=otan

Resolve cockroachdb/django-cockroachdb#54.

This PR involves my audit of eval.go and builtins.go to make sure all
TimestampTZ operations are performed in the context timezone for
TimestampTZ. Results are described in the release note.

Important to note we can't parse everything as the current context
location, because it can be subject to change mid-session.

Release note (bug fix, sql change): Previously, some TimestampTZ
operations did not correctly take context timezone (set by `SET TIME
ZONE`) into account.

This caused a few bugs:
* it leads to bugs involving daylight saving in arithmetic,
e.g. with `America/Chicago`, evaluating
'2010-11-06 23:59:00-05'::timestamptz + '1 day'::interval would return
incorrect results as it assumed it was a fixed offset of `-5` instead.
* text conversion from timestamptz to string sometimes used the wrong
timezone offset if the location of the session does not match the
location when the timestamptz was parsed.
* to_json builtins with timestamptz does not take session timezone
into consideration.

These have all been fixed by the PR.

44819: sql: refactor system.namespace calls to use indexes where available r=solongordon a=otan

Resolves #44230.

Refactoring all system.namespace calls such that they always use indexes
when querying data if it previously used indexes before.

I left `crdb_internal.lookup_namespace_id` as functional as it was
before in that it does not lookup anything that isn't a public schema
or database. This is because the admin UI only looks up tables into the
public schema.

Release note: None



Co-authored-by: Oliver Tan <[email protected]>
@craig
Copy link
Contributor

craig bot commented Feb 10, 2020

Build succeeded

@craig craig bot merged commit 9f884c4 into cockroachdb:master Feb 10, 2020
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.

sql: ensure queries using system.namespace do not degrade with migration
3 participants