Skip to content

moved STATUS and VARIABLES to non_reserved_keyword#3577

Closed
slanning wants to merge 26 commits intovitessio:masterfrom
slanning:non-reserved-keywords
Closed

moved STATUS and VARIABLES to non_reserved_keyword#3577
slanning wants to merge 26 commits intovitessio:masterfrom
slanning:non-reserved-keywords

Conversation

@slanning
Copy link
Copy Markdown
Contributor

Being in reserved_keyword was making queries using status as a column impossible,
so I moved STATUS to non_reserved_keyword.
I also moved VARIABLES since I'd guess it's the same situation.

I wasn't sure about GLOBAL and SESSION, so I left them for now,
but possibly they also should be in non_reserved_keyword. (?)

demmer and others added 10 commits January 18, 2018 20:31
When PassthroughDMLs is set, then all update or delete queries are
returned as PlanPassDML regardless of query type.
Add a flag to the queryserver config that both configures the
planbuilder to use PlanPassDML for all plans and allows those plans
to execute even in statement based replication.
Since PlanPassDML executions are fundamentally unsafe in SBR mode,
they should not be allowed unless the user explicitly wants to
enable them, so add a separate flag which is not exposed in a command
line flag (but will be in vtqueryserver) to allow the unsafe
statements when in SBR mode.
This needs to be added to the Docker Hub auto build process
- Allows for custom database images
- Supports MySQL protocol
- Upgrade to etcd2 topology server
- Uses etcd-operator
- Default pod affinity/anti-affinity
- HPA for vtgates
- Pod security context doesn’t allow root privileges
- Supports default credential chains for GCS/S3
- Moves backup and other config to ConfigMap
- Temporarily removes Orchestrator support
- Vitess components log to stdout/stderr only
- MySQL slow/error logs redirected to stdout
- Removed syslog and hostPath mounts
- Uses new vitess:k8s Docker image
- Support regional failure domain affinity
Add servenv.ParseFlags and servenv.ParseFlagsAndArgs to handle the
common logic around parsing flags, handling the -version flag to
print version, and verify that positional arguments either are or
are not supplied as expected.
This is related to #3520

Also ignore /*!50708 mysql-version-specific */ comments.

I'm not sure if it's a good idea. Instead of stripping leading comments,
in case it's /*! ... */ we don't strip it, and consider it a new StmtComment statement type.
(Not sure if that should be added to ast.go . I didn't.)
I assumed this kind of comment was the only thing in the query,
so if there's something like "/*! ... */ select ..." it wouldn't work.
The handleComment in executor.go basically does nothing, returning &sqltypes.Result{}
Being in reserved_keyword was making queries using status as a column impossible.
I also moved VARIABLES because I'd guess it's the same situation.
I wasn't sure about GLOBAL and SESSION so I left them for now
but possibly they also should be in non_reserved_keyword.
@sougou
Copy link
Copy Markdown
Contributor

sougou commented Jan 23, 2018

Oops! Looks like we encountered a race condition with @dweitzman. Can you resolve the conflicts? We should still get your variables change in.

@bbeaudreault
Copy link
Copy Markdown
Contributor

also, does travis check for shift/reduce conflicts? hard to catch those in review but that's the reason for the reserved_keyword vs non_reserved_keyword. We had tried to put all of the keywords in the most permissive bucket possible. But I see these 2 are recently new so 🤞 no conflicts.

@sougou
Copy link
Copy Markdown
Contributor

sougou commented Jan 23, 2018

Thanks for the warning @bbeaudreault. I just checked it, and we have introduced conflicts. Looks like we'll need to rework the other PR.

@bbeaudreault
Copy link
Copy Markdown
Contributor

No prob. Wonder if the travis script can be enhanced to scrape for the conflict message output from make

@sougou
Copy link
Copy Markdown
Contributor

sougou commented Jan 23, 2018

It looks like this change is good after all. So, I'll just wait for Scott to fix the merge conflicts, and merge this. This should fix the previously introduced shit/reduce conflicts.

enisoc and others added 6 commits January 23, 2018 13:32
ignore mysql-specific comment statements
If we are in autocommit mode and vtgate does not break a DML into
smaller parts, then it has the opportunity to send that statement
through to a vttablet as autocommit in a single round-trip.

Reviewer instructions:
@demmer: I'm not too sure about the vtexplain fixes, or if additional
tests are required there. Extra scrutiny may be required there.

Implementation notes:
* SafeSession has a state machine for tracking autocommit state.
* The autocommit state is initialized by executor as needed.
* VCursor API has been changed for ExecMultiShard. It now accepts
  an extra canCommit flag that should be set to true if the engine
  is executing its final DML. This, combined with the autocommit
  state will decide if an instant autocommit is possible.
v3: instant-commit for autocommit
Being in reserved_keyword was making queries using status as a column impossible.
I also moved VARIABLES because I'd guess it's the same situation.
I wasn't sure about GLOBAL and SESSION so I left them for now
but possibly they also should be in non_reserved_keyword.
@googlebot
Copy link
Copy Markdown

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@slanning
Copy link
Copy Markdown
Contributor Author

I messed that up, didn't I....

@sougou
Copy link
Copy Markdown
Contributor

sougou commented Jan 24, 2018

LOL. You've angered Google's CLA bot.

Scott Lanning added 2 commits January 24, 2018 18:51
Being in reserved_keyword was making queries using status as a column impossible.
I also moved VARIABLES because I'd guess it's the same situation.
I wasn't sure about GLOBAL and SESSION so I left them for now
but possibly they also should be in non_reserved_keyword.
@slanning
Copy link
Copy Markdown
Contributor Author

Sorry, this is probably simple to fix but I'm not getting it and it's late, so I'm just going to make a new PR...

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.

8 participants