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: CREATE TABLE ... AS in implicit transaction does not use the correct sessiondata #73249

Closed
Heoric opened this issue Nov 29, 2021 · 12 comments
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. no-issue-activity O-community Originated from the community T-disaster-recovery X-stale

Comments

@Heoric
Copy link

Heoric commented Nov 29, 2021

Describe the problem

set time zone (and other session variables) are not respected when using CREATE TABLE ... AS

To Reproduce

root@localhost:26257/defaultdb> select current_time;
     current_time
----------------------
  08:56:41.935852+00
(1 row)


Time: 1ms total (execution 1ms / network 1ms)


-- set timezone
root@localhost:26257/defaultdb> set time zone -5;
SET


Time: 2ms total (execution 0ms / network 1ms)

-- implicit txn
root@localhost:26257/defaultdb> create table zone as select current_time;
CREATE TABLE AS


Time: 524ms total (execution 523ms / network 0ms)

root@localhost:26257/defaultdb> select * from zone;
     current_time
----------------------
  08:57:09.304304+00
(1 row)


Time: 69ms total (execution 68ms / network 0ms)



-- explicit txn
root@localhost:26257/defaultdb> begin;
BEGIN


Time: 0ms total (execution 1ms / network 0ms)

root@localhost:26257/defaultdb  OPEN> create table zone1 as select current_time;
CREATE TABLE AS


Time: 25ms total (execution 25ms / network 0ms)

root@localhost:26257/defaultdb  OPEN> select * from zone1;
     current_time
----------------------
  03:58:47.277947-05
(1 row)


Time: 3ms total (execution 3ms / network 0ms)

Jira issue: CRDB-11496

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

blathers-crl bot commented Nov 29, 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 was unable to automatically find someone to ping.

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-untriaged blathers was unable to find an owner labels Nov 29, 2021
@stevendanna stevendanna changed the title when not using explicit transactions, set time zone is invalid when not using explicit transactions, set time zone is invalid Nov 29, 2021
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Nov 30, 2021
@rafiss
Copy link
Collaborator

rafiss commented Dec 3, 2021

Thanks for this report! I am pretty sure the problem is with CREATE TABLE ... AS specifically.

@rafiss rafiss removed the X-blathers-untriaged blathers was unable to find an owner label Dec 3, 2021
@rafiss rafiss changed the title when not using explicit transactions, set time zone is invalid sql: CREATE TABLE ... AS in implicit transaction does not use the correct sessiondata Dec 3, 2021
@rafiss
Copy link
Collaborator

rafiss commented Dec 3, 2021

I believe the problem is that the sessiondata is initialized to use defaults in this part of the schema changer

p, cleanup := NewInternalPlanner(
desc,
txn,
security.RootUserName(),
&MemoryMetrics{},
sc.execCfg,
sessiondatapb.SessionData{},

I'm not sure how the sessiondata can be plumbed down to here, so moving this over to sql-schema.

@rafiss rafiss removed the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Dec 3, 2021
@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Dec 3, 2021
@Heoric
Copy link
Author

Heoric commented Dec 6, 2021

alter table zone add column c time default current_time();

Also have this problem.

@ajwerner
Copy link
Contributor

ajwerner commented Dec 7, 2021

I'm inclined to move this over to @cockroachdb/bulk-io, though I have a feeling they'll be sad to have it. They have more knowledge about working with this sort of thing than anybody on schema.

@blathers-crl
Copy link

blathers-crl bot commented Dec 7, 2021

cc @cockroachdb/bulk-io

@dt
Copy link
Member

dt commented Dec 7, 2021

Hm. Perhaps the SQL statements should refuse to create jobs if the session has non-default values, since they know that the job operates outside a session and thus won't match what the session would when evaluating some expressions?

@rafiss
Copy link
Collaborator

rafiss commented Dec 7, 2021

If the error could also include a hint that it would work as desired if you wrap it with BEGIN; ... COMMIT;, and we document a known limitation, then that might be an acceptable solution. Though potentially, that might mean rejecting all CTAS statements in implicit transactions, because of #73498

@ajwerner
Copy link
Contributor

ajwerner commented Dec 7, 2021

Though potentially, that might mean rejecting all CTAS statements in implicit transactions, because of #73498

We (somebody at CRL before me) wrote a lot of code to support CTAS in implicit transactions writing inside an async job.

@ajwerner
Copy link
Contributor

ajwerner commented Dec 7, 2021

All of these problems seem to equally affect CREATE MATERIALIZED VIEW

@ajwerner
Copy link
Contributor

ajwerner commented Dec 7, 2021

We could serialize the session data?

@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!

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. no-issue-activity O-community Originated from the community T-disaster-recovery X-stale
Projects
No open projects
Archived in project
Development

No branches or pull requests

4 participants