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: more assorted telemetry fixes #32556

Merged
merged 16 commits into from Nov 26, 2018

Conversation

Projects
None yet
4 participants
@knz
Copy link
Member

knz commented Nov 22, 2018

Fixes #31990. Requested by @awoods187.

@knz knz added this to Triage in SQL Front-end, Lang & Semantics via automation Nov 22, 2018

@knz knz requested review from cockroachdb/core-prs as code owners Nov 22, 2018

@cockroach-teamcity

This comment has been minimized.

Copy link
Member

cockroach-teamcity commented Nov 22, 2018

This change is Reviewable

@knz knz force-pushed the knz:20181122-issue-numbrs branch from 872e2e7 to dfde914 Nov 22, 2018

@knz knz requested a review from cockroachdb/sql-bulk-prs as a code owner Nov 22, 2018

@knz knz changed the title [WIP] sql: more assorted telemetry fixes sql: more assorted telemetry fixes Nov 22, 2018

@knz knz requested review from mjibson and BramGruneir Nov 22, 2018

@knz knz force-pushed the knz:20181122-issue-numbrs branch from 859ffd8 to c1fd2dc Nov 22, 2018

@knz knz moved this from Triage to Current milestone in SQL Front-end, Lang & Semantics Nov 22, 2018

@BramGruneir
Copy link
Member

BramGruneir left a comment

:lgtm:

with some minor comments

Reviewed 2 of 2 files at r1, 2 of 2 files at r2, 3 of 3 files at r3, 2 of 2 files at r4, 8 of 8 files at r5, 2 of 2 files at r6, 2 of 2 files at r7, 1 of 1 files at r8, 1 of 1 files at r9, 2 of 2 files at r10, 1 of 1 files at r11, 2 of 2 files at r12, 2 of 2 files at r13, 3 of 3 files at r14, 3 of 3 files at r15, 1 of 1 files at r16.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained


pkg/ccl/importccl/read_import_mysql.go, line 614 at r8 (raw file):

		def.Type = coltypes.JSON

	case mysqltypes.Set:

Is there a test for these?


pkg/sql/sequence.go, line 245 at r16 (raw file):

				// Do nothing; this is the default.
			case v > 1:
				return pgerror.UnimplementedWithIssueErrorf(32567,

tests for these?


pkg/sql/parser/sql.y, line 6812 at r15 (raw file):

opt_timezone:
  WITH_LA TIME ZONE	{ $$.val = true; }

leftover tabs

Show resolved Hide resolved pkg/sql/parser/sql.y Outdated

knz added some commits Nov 22, 2018

sql: reference #9851 for ALTER SET TYPE + detail conversion
The telemetry data did not include the requested type conversion. This
patch adds it.

Release note: None
sql: do not ignore the WHERE clause after INSERT ON CONFLICT
Release note (bug fix): CockroachDB previously silently and
incorrectly accepted then ignored a WHERE clause after INSERT ON
CONFLICT. This is now properly recognized and will produce an error
reporting the feature is unimplemented.

knz added some commits Nov 22, 2018

sql: reference #32098 and #32565 for time/timestamp with precision
This also somewhat simplifies the grammar.

Release note: None

@knz knz force-pushed the knz:20181122-issue-numbrs branch from c1fd2dc to 31d94f1 Nov 26, 2018

@knz
Copy link
Member

knz left a comment

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained


pkg/ccl/importccl/read_import_mysql.go, line 614 at r8 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

Is there a test for these?

Filed #32592 to follow up.


pkg/sql/sequence.go, line 245 at r16 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

tests for these?

Yes they were present previously already in logic_test/sequences: see statement error pgcode 0A000 ...


pkg/sql/parser/sql.y, line 6812 at r15 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

leftover tabs

Done.

@knz

This comment has been minimized.

Copy link
Member

knz commented Nov 26, 2018

TFYRs!

bors r+

craig bot pushed a commit that referenced this pull request Nov 26, 2018

Merge #32556
32556: sql: more assorted telemetry fixes r=knz a=knz

Fixes #31990. Requested by @awoods187.

Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
@craig

This comment has been minimized.

Copy link

craig bot commented Nov 26, 2018

Build succeeded

@craig craig bot merged commit 31d94f1 into cockroachdb:master Nov 26, 2018

3 checks passed

GitHub CI (Cockroach) TeamCity build finished
Details
bors Build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@knz knz deleted the knz:20181122-issue-numbrs branch Nov 26, 2018

@knz knz moved this from Current milestone to Finished (m2.2-2) in SQL Front-end, Lang & Semantics Nov 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment