Fix aggregation on outer joins #12298

harshit-gangal · 2023-02-09T13:39:15Z

Description

This PR fixes an issue with aggregation on the left join by adding coalesce on the result from the right-hand side table.

Related Issue(s)

Fixes Bug Report: bad count result of cross shard outer join #12269

Checklist

"Backport to:" labels have been added if this change should be back-ported 14.0 15.0 16.0
Tests were added or are not required
Documentation was added or is not required

Deployment Notes

vitess-bot · 2023-02-09T13:39:19Z

go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go

go/vt/vtgate/planbuilder/horizon_planning.go

Signed-off-by: Harshit Gangal <harshit@planetscale.com>

vitess-bot · 2023-02-10T15:25:52Z

I was unable to backport this Pull Request to the following branches: release-14.0, release-15.0.

* fix: aggregation coming from right hand side of left join check for null Signed-off-by: Harshit Gangal <harshit@planetscale.com> * test: added a sum test Signed-off-by: Harshit Gangal <harshit@planetscale.com> * addressed review comments Signed-off-by: Harshit Gangal <harshit@planetscale.com> --------- Signed-off-by: Harshit Gangal <harshit@planetscale.com>

* fix: aggregation coming from right hand side of left join check for null Signed-off-by: Harshit Gangal <harshit@planetscale.com> * test: added a sum test Signed-off-by: Harshit Gangal <harshit@planetscale.com> * addressed review comments Signed-off-by: Harshit Gangal <harshit@planetscale.com> --------- Signed-off-by: Harshit Gangal <harshit@planetscale.com> Co-authored-by: Harshit Gangal <harshit@planetscale.com>

* fix: aggregation coming from right hand side of left join check for null * test: added a sum test * addressed review comments --------- Signed-off-by: Harshit Gangal <harshit@planetscale.com>

* Remove replication manager and run VTOrc in all e2e tests (#12149) * feat: remove replication manager Signed-off-by: Manan Gupta <manan@planetscale.com> * test: run VTOrc in all e2e tests (some exceptions) Signed-off-by: Manan Gupta <manan@planetscale.com> * docs: add summary notes changes Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: deprecate disable-replication-manager flag Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix start vtorc function to be idempotent Signed-off-by: Manan Gupta <manan@planetscale.com> * docs: fix review comments Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix port used by vtorc Signed-off-by: Manan Gupta <manan@planetscale.com> * test: really fix the ports issue Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix upgrade downgrade test issues with running vtorc Signed-off-by: Manan Gupta <manan@planetscale.com> * test: run ISP before starting VTOrc Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix flakines in throttler test due to vtorc Signed-off-by: Manan Gupta <manan@planetscale.com> * test: fix flakiness in vreplication tests Signed-off-by: Manan Gupta <manan@planetscale.com> --------- Signed-off-by: Manan Gupta <manan@planetscale.com> * Allow manually kicking off CodeQL (#12200) * Move vtorc from go-sqlite3 to modernc.org/sqlite (#12214) * Move vtorc from go-sqlite3 to modernc.org/sqlite This moves vtorc from the go-sqlite3 library that uses CGO, to use modernc.org/sqlite which is a pure Go implementation. vtorc is the only component we have to build with CGO but it's causing pain for releases since we need to build it against an old Linux for linking against glibc. Using modernc.org/sqlite allows for using Go only again and makes all Vitess components buildable without CGO. In https://datastation.multiprocess.io/blog/2022-05-12-sqlite-in-go-with-and-without-cgo.html someone ran some basic benchmarks. It shows that the pure Go version can be twice as slow, but the usage of vtorc is very limited and we operate on small datasets, so I think the performance impact purely of a somewhat slower sqlite implementation is negligable. None of this is in a hot query serving path or anything like that, so I have little concern performance wise. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix error handling in RowToArray Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Update the MySQL version used by our Docker images (#12054) * Change default MySQL version of lite and base to mysql80 instead of mysql57 Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Bump MySQL version to 8.0.31 in install_dependencies.sh Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * addition of label to the mysql80 image Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * apply review suggestions Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * bring back mysql80 label Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * add release summary Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> --------- Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Revert "Use ubuntu 20.04 for release builds (#12202)" (#12220) The underlying issue has now been addressed in: https://github.com/vitessio/vitess/pull/12214 This reverts commit 4b3b37d41ec08130d162b5115904108cbb3dc14b. Signed-off-by: Matt Lord <mattalord@gmail.com> * Update release notes summary for the new default MySQL version (#12222) Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Revert changes made in #11960 (#12219) Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Bug fix: Cache filtered out tablets in topology watcher to avoid unnecessary GetTablet calls to topo (#12194) * check filter later in loadTablets. Add tests to confirm expected behavior Signed-off-by: Brian Ramos <brirams@users.noreply.github.com> * remove unnecessary assignment Co-authored-by: Florent Poinsard <35779988+frouioui@users.noreply.github.com> Signed-off-by: Brian Ramos <brirams@users.noreply.github.com> * add some explanation around TestFilterByKeypsaceSkipsIgnoredTablets Signed-off-by: Brian Ramos <brirams@users.noreply.github.com> * simplify filter check logic as well as major test cleanup, using testify where possible Signed-off-by: Brian Ramos <brirams@users.noreply.github.com> --------- Signed-off-by: Brian Ramos <brirams@users.noreply.github.com> Co-authored-by: Florent Poinsard <35779988+frouioui@users.noreply.github.com> * Update Go dependencies (#12215) * Update Go dependencies This updates the Go dependencies to the latest versions before the Vitess 16 freeze so we can start with an up to date list. Also updates vitess-mixin since Dependabot is reporting a bunch of things on that. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix deprecated sets.String This is now available using generics. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * run make proto Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * fix e2e test expectations Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Co-authored-by: Florent Poinsard <florent.poinsard@outlook.fr> * Properly test healthcheck cache response handling. (#12226) Signed-off-by: Matt Lord <mattalord@gmail.com> * vtgate advertised mysql server version to 8.0.31 (#11989) * change vtgate advertised mysql version to 8.0.31 Signed-off-by: Harshit Gangal <harshit@planetscale.com> * refactor keeping mysql advertised version Signed-off-by: Harshit Gangal <harshit@planetscale.com> * refactor sqlparser mysql version to use the mysql_server_version default Signed-off-by: Harshit Gangal <harshit@planetscale.com> * test: update test setup Signed-off-by: Harshit Gangal <harshit@planetscale.com> * test: updated test help output Signed-off-by: Harshit Gangal <harshit@planetscale.com> * added release notes Signed-off-by: Harshit Gangal <harshit@planetscale.com> * feat: remove panic since it is getting triggered from the sidecardb Signed-off-by: Manan Gupta <manan@planetscale.com> --------- Signed-off-by: Harshit Gangal <harshit@planetscale.com> Signed-off-by: Andres Taylor <andres@planetscale.com> Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Signed-off-by: Manan Gupta <manan@planetscale.com> Co-authored-by: Andres Taylor <andres@planetscale.com> Co-authored-by: Florent Poinsard <florent.poinsard@outlook.fr> Co-authored-by: Manan Gupta <manan@planetscale.com> * schemadiff: fix scenario where no tables exist in schema and with just views reading from DUAL (#12189) * [Gen4] Fix lookup vindexes with `autocommit` enabled (#12172) * test: show that `autocommit` on lookup vindexes is broken when used after a savepoint is created Signed-off-by: Arthur Schreiber <arthurschreiber@github.com> * test: rework the test case to show that the problem lies with transactions Signed-off-by: Arthur Schreiber <arthurschreiber@github.com> * Correctly handle `autocommit` in the `VIndexLookup` primitive. Signed-off-by: Arthur Schreiber <arthurschreiber@github.com> * Copy the `logging` struct so autocommit queries show up correctly in `vexplain` output. Signed-off-by: Arthur Schreiber <arthurschreiber@github.com> --------- Signed-off-by: Arthur Schreiber <arthurschreiber@github.com> * Add TOC to the summary docs (#12225) * docs: add toc to the summary docs Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: add link to newly added summary change Signed-off-by: Manan Gupta <manan@planetscale.com> --------- Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: bump reported version of main (#12234) Signed-off-by: Manan Gupta <manan@planetscale.com> * Use consistent type for vreplication id (#12218) * Use consistent type for vreplication id The `_vt.vreplication` table has an `INT` type without `UNSIGNED`, so it maps to the Go `int32` type to have the exact same consistent range. This means that we should use `int32` consistently for everywhere that we use the VReplication identifier. We had int32, uint32 and int used pretty randomly for these which leads to CodeQL flagging these and how this was found. Instead this now converts everything to int32. It's also safe to change the protobuf, since the protobuf wire format for int32, uint32, int64 & uint64 is all compatible and actually uses the same variable int length format. The only exception would be if someone has a VReplication identifier that would not fit in `int32`, but well, this PR shows that in that case we already blow up in unexpected and unknown ways anyway. So it's better to have that explicitly error. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Add support for all integer types for bind variables This ensures we cover all the Go int types to generate bind parameters from. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Regenerate protos for vtadmin Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix test for using ParseInt Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix typecasting issue for workflow types (#12217) * Fix typecasting issue for workflow types This addresses a number of the issues identified by CodeQL around typecasting. We use 32 bit integers for these, so let's make sure we parse and cast then as those everywhere it matters. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Ensure to write to int into query Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Use the correct uint32 type for the GTID file position (#12229) This is defined as uint32 at the protocol level, so better to use that type consistently and also parse it accordingly with `ParseUint` with the right size. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix CodeQL identified issues (#12199) This addresses two classes of issues that CodeQL found. One is a not as strict as needed regexp for a domain match, where there's no literal `.` being matched. The other is a user controlled XSS since we run the template with arbitrary user input. Instead we validate if the throttler with that name actually exists before trying to execute the template. On error we also don't reflect the input to avoid the same XSS. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Copy remaining query serving design docs (#12170) * Copy remaining query serving design docs Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Remove duplicate design docs Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> --------- Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Use a working MariaDB mirror for CI (#12253) Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Update the release instructions after v16 code freeze (#12245) Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * onlineddl_vrepl suite: fix auto_increment flakyness (#12246) * onlineddl_vrepl suite: fix auto_increment flakyness Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * delete all rows before events begin Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Revert default MySQL 80 version to `8.0.30` (#12252) * Revert default MySQL 80 version Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * upgrade mysql80 default to 8.0.30 Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Add release notes regarding MySQL upgrade path in the vitess-operator Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Remove unwanted upgrade path step Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> --------- Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Fix additional integer type handling (#12237) * Fix additional integer type handling The tablet uid can be a uint32 everywhere and we can parse it like that with pflag as well. This way we don't have to cast and change the valid range and get correct messages when it's out of range. As well we use a regular int for the port which better matches how Go itself tracks ports and removes a bunch of casts from places where we already using int. Lastly it also fixes some small cases of how we parse to match the type we store it in. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix parsing of server id for Zookeeper Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix help output Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * grpcvtgateconn: add Dial and deprecate DialWithOpts (#12210) Signed-off-by: Matt Layher <mdlayher@planetscale.com> Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Co-authored-by: Dirkjan Bussink <d.bussink@gmail.com> * Use consistent type port and replication state (#12248) * Use consistent type port and replication state These are used as int32 in the protobuf (and enum maps to int32 for replication state). This means we should also keep using int32 internally if easily possible. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix assertions in test Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * evalengine: new evaluation framework (#12247) * evalengine: new evaluation framework Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: add cached_size.go Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: simplify casts in NullSafeHashCodeInPlace Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: fix static warnings Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: remove duplicate branch Signed-off-by: Vicent Marti <vmg@strn.cat> * planbuilder: update for tuple bind variables Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: fix error in tuple hashing Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: fix LIKE simplification Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: cleanup unused tests and panics Signed-off-by: Vicent Marti <vmg@strn.cat> --------- Signed-off-by: Vicent Marti <vmg@strn.cat> * Examples: Add lib functions to wait for shard states (#12239) * Add lib functions to wait for shard states Signed-off-by: Matt Lord <mattalord@gmail.com> * Fixup region example too Signed-off-by: Matt Lord <mattalord@gmail.com> * Address minor review comments Signed-off-by: Matt Lord <mattalord@gmail.com> * Remove extraneous line change Signed-off-by: Matt Lord <mattalord@gmail.com> * Refactor to remove duplicated files/code Also address shellcheck warnings. Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix tests that run in CI Signed-off-by: Matt Lord <mattalord@gmail.com> * Use more robust location detection Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix backup tests Signed-off-by: Matt Lord <mattalord@gmail.com> * Exit on shard healthy failure Signed-off-by: Matt Lord <mattalord@gmail.com> * Remove unused dups and adjust client_test and Dockerfiles Signed-off-by: Matt Lord <mattalord@gmail.com> --------- Signed-off-by: Matt Lord <mattalord@gmail.com> * Download mariadb from vitess-resources (#12271) * Download mariadb from vitess-resources Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Use proper mariadb version increment in tests Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Use 10.10.3 instead of LTS Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> --------- Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Workflow Show: display rows copied (#12231) * Display rows copied in Workflow Show Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Fix vtctl tests Signed-off-by: Rohit Nayak <rohit@planetscale.com> --------- Signed-off-by: Rohit Nayak <rohit@planetscale.com> * sqlparser: Add parsing support to spatial POINT expression (#12198) * feat: add parsing support to spatial POINT expression Signed-off-by: Ayman161803 <mohammedaymannawaz@gmail.com> * fix: address formatting issues Signed-off-by: Ayman161803 <mohammedaymannawaz@gmail.com> * fix: address formatting issues Signed-off-by: Ayman <76606666+Ayman161803@users.noreply.github.com> * feat: add canonical test cases for spatial point expression Signed-off-by: Ayman161803 <mohammedaymannawaz@gmail.com> --------- Signed-off-by: Ayman161803 <mohammedaymannawaz@gmail.com> Signed-off-by: Ayman <76606666+Ayman161803@users.noreply.github.com> * fix dual table handling (#12204) * fix planner: be careful when merging subqueries We were not checking dependencies of the outer subquery correctly, before merging with a route. This lead to invalid plans being produced. Signed-off-by: Andres Taylor <andres@planetscale.com> * fix planner: use tables on lhs when merging subqs When calculating if everything we need is available before merging routes, we now take into account anything coming from the LHS of any joins we are under. Signed-off-by: Andres Taylor <andres@planetscale.com> * test: add test merging subquery with join Signed-off-by: Andres Taylor <andres@planetscale.com> * use LHS from the top of the current node when planning RHSs Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * added comments Signed-off-by: Andres Taylor <andres@planetscale.com> --------- Signed-off-by: Andres Taylor <andres@planetscale.com> Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Co-authored-by: Florent Poinsard <florent.poinsard@outlook.fr> * Subquery bugfix (#12254) * fix planner: be careful when merging subqueries We were not checking dependencies of the outer subquery correctly, before merging with a route. This lead to invalid plans being produced. Signed-off-by: Andres Taylor <andres@planetscale.com> * fix planner: use tables on lhs when merging subqs When calculating if everything we need is available before merging routes, we now take into account anything coming from the LHS of any joins we are under. Signed-off-by: Andres Taylor <andres@planetscale.com> * test: add test merging subquery with join Signed-off-by: Andres Taylor <andres@planetscale.com> * use LHS from the top of the current node when planning RHSs Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * added comments Signed-off-by: Andres Taylor <andres@planetscale.com> --------- Signed-off-by: Andres Taylor <andres@planetscale.com> Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Co-authored-by: Florent Poinsard <florent.poinsard@outlook.fr> * feat: fix release script (#12284) Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: copy release notes from release-16 (#12287) Signed-off-by: Manan Gupta <manan@planetscale.com> * Remove parallelization from tests that mutate shared state (#12257) Signed-off-by: Andrew Mason <andrew@planetscale.com> Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Co-authored-by: Florent Poinsard <florent.poinsard@outlook.fr> * fix dual table handling (#12292) * fix dual table handling Signed-off-by: Andres Taylor <andres@planetscale.com> * update test expectations Signed-off-by: Andres Taylor <andres@planetscale.com> --------- Signed-off-by: Andres Taylor <andres@planetscale.com> * fix: transaction_isolation to be applied at session level (#12281) Signed-off-by: Harshit Gangal <harshit@planetscale.com> * fix linter warnings (#12286) * fix linter warnings Signed-off-by: Andres Taylor <andres@planetscale.com> * add the same checks in CI as pre-commit Signed-off-by: Andres Taylor <andres@planetscale.com> * ci: disable revive by default Signed-off-by: Vicent Marti <vmg@strn.cat> --------- Signed-off-by: Andres Taylor <andres@planetscale.com> Signed-off-by: Vicent Marti <vmg@strn.cat> Co-authored-by: Vicent Marti <vmg@strn.cat> * go/vt/mysqlctl: add configurable read buffer to builtin backups (#12073) * go/vt/mysqlctl: add configurable read buffer to builtin backups Signed-off-by: Max Englander <max@planetscale.com> * pr review: use uint Signed-off-by: Max Englander <max@planetscale.com> * doc/releasenotes: v16 --builtinbackup-file-read-buffer-size and --builtinbackup-file-write-buffer-size Signed-off-by: Max Englander <max@planetscale.com> * update flags txt Signed-off-by: Max Englander <max@planetscale.com> * make part of v17 Signed-off-by: Max Englander <max@planetscale.com> * Update doc/releasenotes/17_0_0_summary.md Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com> Signed-off-by: Max Englander <max.englander@gmail.com> * improve wording -block Signed-off-by: Max Englander <max@planetscale.com> --------- Signed-off-by: Max Englander <max@planetscale.com> Signed-off-by: Max Englander <max.englander@gmail.com> Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com> * VReplication Last Error: retry error if it happens after timeout (#12114) * Retry error if it happens after timeout Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Address review comments Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Address review comments Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Improve unit tests Signed-off-by: Rohit Nayak <rohit@planetscale.com> --------- Signed-off-by: Rohit Nayak <rohit@planetscale.com> * fix: added null safe operator precendence rule (#12297) Signed-off-by: Harshit Gangal <harshit@planetscale.com> * Fix aggregation on outer joins (#12298) * fix: aggregation coming from right hand side of left join check for null Signed-off-by: Harshit Gangal <harshit@planetscale.com> * test: added a sum test Signed-off-by: Harshit Gangal <harshit@planetscale.com> * addressed review comments Signed-off-by: Harshit Gangal <harshit@planetscale.com> --------- Signed-off-by: Harshit Gangal <harshit@planetscale.com> * Change SQL error codes to use explicit type (#12249) This ErrorCode type maps to a uint16 to match what MySQL has as the range and parses things accordingly as well. With the explicit type we know better when we use the specific error code. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix typecasting alerts inside vtgate engine (#12238) CodeQL reported a few cases in the vtgate engine where we can simplify and make the typecasting more correct. We were double casting for the eval as well which isn't useful, so reduce this to it doing one for the hash code. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Move to independent sets package (#12251) This moves to a separate sets package inside Vitess so we don't break version pinning for k8s like in the Vitess operator. It mostly copies the existing implementation, but simplifies and cleans it up to only copy what we actually use. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * SchemaDiff: normalize boolean columns to tinyint(1), and, add TableQualifier hint to force adding a db qualifier to the alter diff. (#12133) * Normalize boolean columns to tinyint(1) Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Properly handle invalid defaults provided for boolean columns Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Use TableQualifier hint to decide whether to use the db qualifier for the tables in create/alter scripts Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Keep current behavior as default and require TableQualifierDeclared to force use of the 'to' desired schema's qualifier Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Fix incorrect renaming of enum in comments Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Additional tests for boolean/tinyint(1) equivalence Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Add tests for create/drop with table qualifiers Signed-off-by: Rohit Nayak <rohit@planetscale.com> --------- Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Allow zero (in) date when setting up internal _vt schema (#12262) * Allow zero (in) date when setting up internal _vt schema Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * modify test sto include @@sql_mod query support Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Allow zero (in) date when setting up internal _vt schema Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * modify test sto include @@sql_mod query support Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Fix test failures Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Fix failing tests. Minor refactor Signed-off-by: Rohit Nayak <rohit@planetscale.com> * change sql_mode t omost permissive (empty) Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * setPermissiveSQLMode Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * fixes per review Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Add missing mock query Signed-off-by: Rohit Nayak <rohit@planetscale.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Signed-off-by: Rohit Nayak <rohit@planetscale.com> Co-authored-by: Rohit Nayak <rohit@planetscale.com> * maintainers: add dbussink and rework some sections (#12329) Signed-off-by: Vicent Marti <vmg@strn.cat> * TableFilter: fix excluded RE listing (#12318) Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Internal refactor: LastError as a public struct (#12321) * Refactor LastError as a public struct Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * wording Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * evalengine: it's time for JSON! (#12274) * collations: add max width to charsets Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: add support for JSON built-ins Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: support JSON_ARRAY and JSON_OBJECT Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: JSON object sorting like MySQL Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: add support for CAST(val AS JSON) Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: support TO_BASE64 and FROM_BASE64 Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: support JSON_DEPTH Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: support JSON_LENGTH Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: fix codegen Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: do not wrap `json.Value` Signed-off-by: Vicent Marti <vmg@strn.cat> * sizegen: fix external type generation Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: support JSON_CONTAINS_PATH Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: support JSON_KEYS Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: fix linter warnings Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: fix evalToJSON semantics Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: fix error wrapping Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: update sizegen Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: @systay review feedback Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: oops! fix bad else/if Signed-off-by: Vicent Marti <vmg@strn.cat> --------- Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: fix bugs with decimal rounding (#12360) Signed-off-by: Vicent Marti <vmg@strn.cat> * VReplication: ignore GC tables in schema analysis (#12320) * TableFilter: fix excluded RE listing Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * extract GCTableNameExpression Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * VReplication: exclude GC tables when reading schema Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * wrap schema analyzis errors for better tracability Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Add parsing support for linestring constructor (#12299) * feat: add parsing support to linestring constructor Signed-off-by: Ayman161803 <mohammedaymannawaz@gmail.com> * test: add canonical tests Signed-off-by: Ayman161803 <mohammedaymannawaz@gmail.com> * fix: fix formatting issues Signed-off-by: Ayman161803 <mohammedaymannawaz@gmail.com> * chore: increment shift/reduce conflicts to 4 Signed-off-by: Ayman161803 <mohammedaymannawaz@gmail.com> * fix: fix shift/reduce conflict by adding precendence Signed-off-by: Ayman161803 <mohammedaymannawaz@gmail.com> * fix: update yacc grammar to replace []pointExpr with Exprs Signed-off-by: Ayman161803 <mohammedaymannawaz@gmail.com> * fix: update format function for linestringExpr and select_test_cases Signed-off-by: Ayman161803 <mohammedaymannawaz@gmail.com> --------- Signed-off-by: Ayman161803 <mohammedaymannawaz@gmail.com> * testutils: use a rows parser for test comparisons (#12138) * tesutils: use a rows parser for test comparisons Signed-off-by: Vicent Marti <vmg@strn.cat> * utils: add license Signed-off-by: Vicent Marti <vmg@strn.cat> * moved parse_rows to sqltypes to avoid import cycle Signed-off-by: Andres Taylor <andres@planetscale.com> * concatenate: fix test Signed-off-by: Vicent Marti <vmg@strn.cat> * sqltypes: fix comparison error with duplicate rows Signed-off-by: Vicent Marti <vmg@strn.cat> * vexplain: fix undeterministic test comparisons Signed-off-by: Vicent Marti <vmg@strn.cat> --------- Signed-off-by: Vicent Marti <vmg@strn.cat> Signed-off-by: Andres Taylor <andres@planetscale.com> Co-authored-by: Andres Taylor <andres@planetscale.com> * vtbackup, mysqlctl: detailed backup and restore metrics (#11979) * vtbackup, mysqlctl: detailed backup and restore metrics While backup stats are exposed via `servenv` (which is great!) ideally we would like more fine-grained stats on: * Download last backup. * Decompress last backup. * Catchup to primary with replication. * Compress new backup. * Upload new backup. Signed-off-by: Max Englander <max@planetscale.com> * move fakemysqldaemon.go to mysqlctl package Signed-off-by: Max Englander <max@planetscale.com> * go/vt/mysqlctl: add backup/restore stats tests Signed-off-by: Max Englander <max@planetscale.com> * address pr comments round 1 Signed-off-by: Max Englander <max@planetscale.com> * checkpoint Signed-off-by: Max Englander <max@planetscale.com> * reorganize metric names Signed-off-by: Max Englander <max@planetscale.com> * track bytes processed in backup/restore Signed-off-by: Max Englander <max@planetscale.com> * address pr comments: remove 'this file' from comments Signed-off-by: Max Englander <max@planetscale.com> * Update go/vt/mysqlctl/backupstats/stats.go Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com> Signed-off-by: Max Englander <max.englander@gmail.com> * Update go/vt/mysqlctl/backupstats/stats.go Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com> Signed-off-by: Max Englander <max.englander@gmail.com> * Update go/vt/mysqlctl/backupstats/stats.go Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com> Signed-off-by: Max Englander <max.englander@gmail.com> * pr review: improve comments Signed-off-by: Max Englander <max@planetscale.com> * make stats more expvars-friendly Signed-off-by: Max Englander <max@planetscale.com> * -backupstorage.Parameterizer, +backupstorage.BackupStorage.WithParams Signed-off-by: Max Englander <max@planetscale.com> * remove deleted field Signed-off-by: Max Englander <max@planetscale.com> * Update go/ioutil/meter.go Signed-off-by: Max Englander <max.englander@gmail.com> * Update go/ioutil/meter.go Signed-off-by: Max Englander <max.englander@gmail.com> * Update go/ioutil/meter.go Signed-off-by: Max Englander <max.englander@gmail.com> * -constants.go +release notes Signed-off-by: Max Englander <max@planetscale.com> * add stats example to release notes Signed-off-by: Max Englander <max@planetscale.com> --------- Signed-off-by: Max Englander <max@planetscale.com> Signed-off-by: Max Englander <max.englander@gmail.com> Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com> * VReplication: workflows with multiple streams fail to start on multi-primary setups (#12228) * Handle auto_increment_increment step > 1 while creating multiple vreplication streams Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Enhance e2e test for validating auto_increment step logic Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Add new query to mock db Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Add missing mock for TestEngineExec Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Cleanup Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Fix goimports Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Address review comments Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Use uint16 for auto increment step Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Update comments Signed-off-by: Rohit Nayak <rohit@planetscale.com> --------- Signed-off-by: Rohit Nayak <rohit@planetscale.com> * sqlparser/schemadiff: normalize CAST type case (#12372) Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * add database name to _vt.views table (#12374) Signed-off-by: Harshit Gangal <harshit@planetscale.com> Co-authored-by: Harshit Gangal <harshit@planetscale.com> * [vtgate planner] Routing & Merging refactor (#12197) * start of extracting routing logic out from Route into an interface Signed-off-by: Andres Taylor <andres@planetscale.com> * wip - milestone - first plan-test green Signed-off-by: Andres Taylor <andres@planetscale.com> * wip - single table routes work fine Signed-off-by: Andres Taylor <andres@planetscale.com> * wip - sequence tables passing Signed-off-by: Andres Taylor <andres@planetscale.com> * wip - all tests in from_cases.json passing Signed-off-by: Andres Taylor <andres@planetscale.com> * wip - more work on merge logic Signed-off-by: Andres Taylor <andres@planetscale.com> * wip - need to switch the routes as well Signed-off-by: Andres Taylor <andres@planetscale.com> * subquery merging Signed-off-by: Harshit Gangal <harshit@planetscale.com> * dual routing Signed-off-by: Harshit Gangal <harshit@planetscale.com> * wip - make sure to keep inner/outer in the right place Signed-off-by: Andres Taylor <andres@planetscale.com> * make sure to not forget already seen predicates when merging sharded routes Signed-off-by: Andres Taylor <andres@planetscale.com> * add merged subqueries to the merged field Signed-off-by: Andres Taylor <andres@planetscale.com> * dual subquery and none routing change Signed-off-by: Harshit Gangal <harshit@planetscale.com> * handle merging subqueries on IN comparisons Signed-off-by: Andres Taylor <andres@planetscale.com> * recalculate routing after merging subquery Signed-off-by: Andres Taylor <andres@planetscale.com> * rename field to make it easier to grokk Signed-off-by: Andres Taylor <andres@planetscale.com> * better merging logic when merging non-sharded tables Signed-off-by: Andres Taylor <andres@planetscale.com> * when merging subqueries, we must also copy predicates Signed-off-by: Andres Taylor <andres@planetscale.com> * copy keyspace when producing NoneRouting Signed-off-by: Andres Taylor <andres@planetscale.com> * implement Routing methods in TargetedRouting and update OpCode outside of shardedrouting Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Fix targeted routing update params and rest of the dml cases Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Enhanced UpdateRoutingLogic function to detect constant null and None routes Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * clean up info schema route mergeing Signed-off-by: Andres Taylor <andres@planetscale.com> * handle reference tables with alternates Signed-off-by: Andres Taylor <andres@planetscale.com> * update remaining plan_tests Signed-off-by: Andres Taylor <andres@planetscale.com> * unify unsharded and reference routing in the same logic Signed-off-by: Andres Taylor <andres@planetscale.com> * clean up merging.go Signed-off-by: Andres Taylor <andres@planetscale.com> * build the alternate routes with the correct keyspace Signed-off-by: Andres Taylor <andres@planetscale.com> * minor fixes after self-review Signed-off-by: Andres Taylor <andres@planetscale.com> * when missing current keyspace, any valid keyspace can be used Signed-off-by: Andres Taylor <andres@planetscale.com> * final small changes. I promise Signed-off-by: Andres Taylor <andres@planetscale.com> * review feedback Signed-off-by: Andres Taylor <andres@planetscale.com> * tidy up method after review feedback Signed-off-by: Andres Taylor <andres@planetscale.com> * more cleanup - fix goland warnings in new files Signed-off-by: Andres Taylor <andres@planetscale.com> * add keyspace information to sequence routing Signed-off-by: Andres Taylor <andres@planetscale.com> * more cleanups from PR review Signed-off-by: Andres Taylor <andres@planetscale.com> * more cleanup Signed-off-by: Andres Taylor <andres@planetscale.com> --------- Signed-off-by: Andres Taylor <andres@planetscale.com> Signed-off-by: Harshit Gangal <harshit@planetscale.com> Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Co-authored-by: Harshit Gangal <harshit@planetscale.com> Co-authored-by: Florent Poinsard <florent.poinsard@outlook.fr> * This PR bumps the vitess resrouces release increment from v2 to v4 (#12272) Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Switch to using new Go 1.19 CRL parser (#12315) `x509.ParseCRL` is deprecated, we should use `x509.ParseRevocationList` instead which is new in Go 1.19. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Online DDL: remove legacy "stowaway table" logic (#12288) * Online DDL: remove legacy stowaway table logic Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * removed column from table definition Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Fix for USING when column names not lower cased (#12378) Signed-off-by: Andres Taylor <andres@planetscale.com> * Fix bug in vtexplain around JOINs (#12376) Signed-off-by: Andres Taylor <andres@planetscale.com> * fix help text for backup_storage_number_blocks (#12258) Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * Fix additional typecasting alerts (#12309) * Fix additional typecasting alerts This should solve the remaining valid typecasting alerts from CodeQL. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix additional typecasting issues Ran through more of the ParseInt / ParseUint / Atoi calls manually that CodeQL didn't identify and found a bunch more inconsistencies that should be fixed. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix additional casting bugs in the sqlparser This fixes a bunch of additional canonical casing bugs in the parser. See https://github.com/vitessio/vitess/issues/12392 for the list of issues it addresses. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fixes for local docker environment (#12382) Signed-off-by: Andy Lim <alim@duosecurity.com> * [main] Schema RPC to fetch table/view definition (#12375) (#12397) * feat: proto changes to add new rpc Signed-off-by: Harshit Gangal <harshit@planetscale.com> * feat: vttablet changes in queryservice to implement the new rpc Signed-off-by: Harshit Gangal <harshit@planetscale.com> * added test on tabletserver Signed-off-by: Harshit Gangal <harshit@planetscale.com> * feat: use the new schema rpc in vtgate to get updated view definition Signed-off-by: Harshit Gangal <harshit@planetscale.com> * remove error from getschemaresponse and change the enum name Signed-off-by: Harshit Gangal <harshit@planetscale.com> * updated proto files for vtadmin Signed-off-by: Harshit Gangal <harshit@planetscale.com> * fix e2e test Signed-off-by: Harshit Gangal <harshit@planetscale.com> --------- Signed-off-by: Harshit Gangal <harshit@planetscale.com> Co-authored-by: Harshit Gangal <harshit@planetscale.com> * Bump golang.org/x/net from 0.5.0 to 0.7.0 (#12390) Bumps [golang.org/x/net](https://github.com/golang/net) from 0.5.0 to 0.7.0. - [Release notes](https://github.com/golang/net/releases) - [Commits](https://github.com/golang/net/compare/v0.5.0...v0.7.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Move to Go 1.19 atomics (#12391) * Move to Go 1.19 atomics This change moves to Go 1.19 atomics in favor of the version we had in the `sync2` package. The version in that package predates the availability of the new atomics, but for the future we want to use the Go provided ones. On top of that, we can also use the semaphore from `x/sync` instead of a custom one in `sync2`. This cleans up a bunch of additional code from `sync2`. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Don't assume the value is a duration This can be any float, so we need to store the float bits in an atomic uint64 instead (as Go has no atomic floats). Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Log actual error if acquiring semaphore fails Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix overlooked metrics threshold case Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Update additional Go dependencies (#12401) Dependabot does pick up these go.mod files but somehow fails to open PRs for them to update things that are vulnerable. So doing it manually here. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Fix another mixin dep (#12403) Dependabot is still warning about this one and missed this in #12401 Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Remove a bunch of debug logging (#12404) This removes a bunch of debug logging that is set for higher log levels. The problem is that many of these are logging user provided values directly which is something CodeQL also alerts on. If someone needs this kind of additional logging level, I think it suffices to modify the code locally to test something rather than always carrying these high log level entries. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Stop launchable integration for unit tests (#12386) * feat: stop launchable integration for unit tests Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix identation for the unit tests Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: also add name to the unit tests Signed-off-by: Manan Gupta <manan@planetscale.com> --------- Signed-off-by: Manan Gupta <manan@planetscale.com> * Makefile: use buildx when GOOS/GOARCH are specified (#12081) Signed-off-by: Max Englander <max@planetscale.com> * Auto refresh tables on vtgate Status Page (#12335) * Component: vtgate, Auto Update tables in Status For Vtgate page Signed-off-by: jun <johnwang.wangjun@gmail.com> * rename function name Signed-off-by: jun <johnwang.wangjun@gmail.com> * move tableRefreshRate to servenv.RegisterFlags() Signed-off-by: jun <johnwang.wangjun@gmail.com> * Replace JQuery with Vanilla javascript && set default value 0 as non-refreshing && update e2e .txt flags Signed-off-by: jun <johnwang.wangjun@gmail.com> * update go/flags/endtoend/*.txt Signed-off-by: jun <johnwang.wangjun@gmail.com> * update e2e flags txt files Signed-off-by: jun <johnwang.wangjun@gmail.com> * update --table-refresh-interval tag with proper position Signed-off-by: jun <johnwang.wangjun@gmail.com> * remove extra space in mysqlctl.txt vtorc.txt repush Signed-off-by: jun <johnwang.wangjun@gmail.com> --------- Signed-off-by: jun <johnwang.wangjun@gmail.com> Co-authored-by: jun <johnwang.wangjun@gmail.com> * (re)Formalize SQLError in VReplication, add underlying wrap/unwrap functionality (#12327) * (re)Formalize SQLError in VReplication, add underlying wrap/unwrap functionality Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * update test to match new UnwrapAll() behavior Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * do not unwrap Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Online DDL: improve retry of vreplication errors with `vitess` `ALTER TABLE` migrations (#12323) * Refactor LastError as a public struct Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Online DDL: use LastError for retrying vreplication streams Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * differentiate between a temrinally failed workflow and one which 'has error' and can be retried Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * wording Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * simplified hasError() Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Fixing `onlineddl_vrepl` flakiness, and adding more tests (#12325) * TableFilter: fix excluded RE listing Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * extract GCTableNameExpression Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * VReplication: exclude GC tables when reading schema Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * wrap schema analyzis errors for better tracability Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Refactor LastError as a public struct Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Online DDL: use LastError for retrying vreplication streams Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * refactor UpdateThrottlerTopoConfig Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * onlineddl_vrepl tests: - using topo-based throttler config - break down of throttle-based PRS test - adding --postpone-completion -based PRS test - fixing wait for VReplicatoin status Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * differentiate between a temrinally failed workflow and one which 'has error' and can be retried Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * (re)Formalize SQLError in VReplication, add underlying wrap/unwrap functionality Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * differentiate between a temrinally failed workflow and one which 'has error' and can be retried Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * wording Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * simplified hasError() Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * do not unwrap Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Cleanup vtorc ssl package (#12423) This is not used for anything anymore, so let's remove it. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * SidecarDB Init: don't fail on schema init errors (#12328) * Don't fail on schema init errors Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Fix debug var attribute name Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Fix typo in debug var Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Address review comment Signed-off-by: Rohit Nayak <rohit@planetscale.com> * Address review comments Signed-off-by: Rohit Nayak <rohit@planetscale.com> --------- Signed-off-by: Rohit Nayak <rohit@planetscale.com> * [main] Examples: Correct VTAdmin Discovery File Path And Add Check (#12415) (#12430) * Examples: correct VTAdmin discovery file path and add check Signed-off-by: Matt Lord <mattalord@gmail.com> * Remove jq dependency Signed-off-by: Matt Lord <mattalord@gmail.com> * vtadmin examples scripts hardcode localhost Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix errant expectation Signed-off-by: Matt Lord <mattalord@gmail.com> * Adjust healthy shard indicator Signed-off-by: Matt Lord <mattalord@gmail.com> --------- Signed-off-by: Matt Lord <mattalord@gmail.com> Co-authored-by: Matt Lord <mattalord@gmail.com> * sqlparser: Tools cleanup (#12407) * tools: fix compilation checks in ast helpers Signed-off-by: Vicent Marti <vmg@strn.cat> * sqlparser: simplify generation Signed-off-by: Vicent Marti <vmg@strn.cat> * tools: make sizegen more resilient Signed-off-by: Vicent Marti <vmg@strn.cat> * tools: fix formatting of all generated code Signed-off-by: Vicent Marti <vmg@strn.cat> * tools: fix call Signed-off-by: Vicent Marti <vmg@strn.cat> * tools: review feedback Signed-off-by: Vicent Marti <vmg@strn.cat> --------- Signed-off-by: Vicent Marti <vmg@strn.cat> * Add configuration option for minimum TLS version (#12424) * Add configuration option for minimum TLS version We were hardcoding this to TLS 1.0 which is not considered safe anymore these days and was called out by CodeQL as a security issue. Instead we should default to TLS 1.2 which we do everywhere else, and allow for an option to change it if needed so that we're safe by default here. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Add release notes item Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> --------- Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * VReplication: Use MariaDB Compat JSON Functions (#12434) This is needed to better support importing from MariaDB into Vitess/MySQL. Signed-off-by: Matt Lord <mattalord@gmail.com> Co-authored-by: Matt Lord <mattalord@gmail.com> * Remove are-you-alive example (#12432) Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * schemadiff: AlterTableAlgorithmStrategy (#12442) * schemadiff: AlterTableAlgorithmStratgy Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * minor rename Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * Add release notes summary for views (#12422) (#12425) * Add release notes summary for support views * add link to the documentation * simplify views release notes --------- Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * [main] Change `GetSchema` RPC to return `CreateView` instead of `SelectStmt` (#12421) (#12441) * Changed getSchemaRPC to return CreateView instead of SelectStmt Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * test: fix expectation Signed-off-by: Harshit Gangal <harshit@planetscale.com> --------- Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Signed-off-by: Harshit Gangal <harshit@planetscale.com> Co-authored-by: Florent Poinsard <florent.poinsard@outlook.fr> Co-authored-by: Harshit Gangal <harshit@planetscale.com> * Add 128 bit hashing for the evalengine (#12428) * evalengine: 128-bit hashes This adds support for 128 bit hashes. With 128 bit hashes, we have enough entropy so we can remove the need to compare values after hashing. Signed-off-by: Vicent Marti <vmg@strn.cat> Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * evalengine: Add support for 128 bit hashing This adds a null safe hashing function for 128 bit hashing and tests it. There's no usage of this yet, but that will be extended in the future. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Address review feedback Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Ensure to always initialize hasher Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> --------- Signed-off-by: Vicent Marti <vmg@strn.cat> Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Co-authored-by: Vicent Martí <42793+vmg@users.noreply.github.com> * feat: update the pull request template (#12443) Signed-off-by: Manan Gupta <manan@planetscale.com> * evalengine: use 128 bit hashing internally (#12452) * evalengine: use 128 bit hashing internally Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: implement hashable for hashable types Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: update sizegen Signed-off-by: Vicent Marti <vmg@strn.cat> * evalengine: fix error with unsupported collations Signed-off-by: Vicent Marti <vmg@strn.cat> --------- Signed-off-by: Vicent Marti <vmg@strn.cat> * Fix Vtop example and release script (#12457) * examples: should use mysql80 property for vitess image Signed-off-by: deepthi <deepthi@planetscale.com> Signed-off-by: Manan Gupta <manan@planetscale.com> * Use MySQL80 in all examples/operator YAML files Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: update release script to update vtadmin image version too Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix vtop example script to use vtctldclient instead Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: update operator.yaml to the latest version Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: fix yaml files to not pass deprecated parameters Signed-off-by: Manan Gupta <manan@planetscale.com> * feat: revert changes made for testing Signed-off-by: Manan Gupta <manan@planetscale.com> --------- Signed-off-by: deepthi <deepthi@planetscale.com> Signed-off-by: Manan Gupta <manan@planetscale.com> Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Co-authored-by: deepthi <deepthi@planetscale.com> Co-authored-by: Florent Poinsard <florent.poinsard@outlook.fr> * feat: add steps on how to merge during code-freeze (#12444) Signed-off-by: Manan Gupta <manan@planetscale.com> * [main] GetSchema rpc to streaming api (#12447) (#12459) * GetSchema rpc to streaming api Signed-off-by: Harshit Gangal <harshit@planetscale.com> * addressed review comment Signed-off-by: Harshit Gangal <harshit@planetscale.com> --------- Signed-off-by: Harshit Gangal <harshit@planetscale.com> Signed-off-by: Manan Gupta <manan@planetscale.com> Co-authored-by: Harshit Gangal <harshit@planetscale.com> Co-authored-by: Manan Gupta <manan@planetscale.com> * feat: don't keep data for upgrade-downgrade tests (#12465) Signed-off-by: Manan Gupta <manan@planetscale.com> Co-authored-by: Manan Gupta <manan@planetscale.com> * Create Views allowed for same keyspace (#12409) (#12454) * restrict views to single keyspace * feat: also create/alter and drop views on MySQL for information-schema-support * feat: do the DMLs for alter and create of view in a transaction so that they can be rolled back in case of MySQL failure * ignore view changes by schema tracker in vttablet when views are enabled * test: fix test expectation * Correct syntax error in defer func of E2E test --------- Signed-off-by: Harshit Gangal <harshit@planetscale.com> Signed-off-by: Manan Gupta <manan@planetscale.com> Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Co-authored-by: Manan Gupta <manan@planetscale.com> Co-authored-by: Florent Poinsard <florent.poinsard@outlook.fr> * Add port to enable vtadmin access. (#12467) Signed-off-by: Andy Lim <alim@duosecurity.com> * Fix scalar aggregation engine primitive for column truncation (#12468) (#12473) * fix: scalar aggregation truncation * test: added scalar aggr engine unit test * remove onecase change --------- Signed-off-by: Harshit Gangal <harshit@planetscale.com> Co-authored-by: Harshit Gangal <harshit@planetscale.com> * Fix `SHOW VSCHEMA TABLES` tests using v17 vtgate that expected `dual` (#12381) (#12466) * Fix SHOW VSCHEMA TABLES tests using v17 vtgate * Fix tests expectations --------- Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Signed-off-by: Manan Gupta <manan@planetscale.com> Co-authored-by: Florent Poinsard <35779988+frouioui@users.noreply.github.com> * examples schema: demi-idempotency via CREATE TABLE IF NOT EXISTS (#12453) * examples schema: demi-idempotency via CREATE TABLE IF NOT EXISTS Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * idempotency: if not exists for all tables under examples/ Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * evalengine: add more coercion paths to Hash128 (#12463) Signed-off-by: Vicent Marti <vmg@strn.cat> * [misc] Delete `automation*` protos (#12449) * Remove unused automation,automationservice protos and codegen Signed-off-by: Andrew Mason <andrew@planetscale.com> * Release notes Signed-off-by: Andrew Mason <andrew@planetscale.com> --------- Signed-off-by: Andrew Mason <andrew@planetscale.com> * BugFix: Unsharded query using a derived table and a dual table (#12484) (#12491) * test: add failing test for a query in an unsharded keyspace using a derived table and reference table * feat: fix logic to check if a single unsharded keyspace is used * feat: fix test output --------- Signed-off-by: Manan Gupta <manan@planetscale.com> * Make upgrade downgrade job names unique (#12485) Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Add nvm and node to vtadmin-up.sh (#12439) * Add nvm and node Signed-off-by: notfelineit <notfelineit@gmail.com> * Update script with PR recommendations Signed-off-by: notfelineit <notfelineit@gmail.com> * Push up local changes Signed-off-by: notfelineit <notfelineit@gmail.com> * Update examples/common/scripts/vtadmin-up.sh Signed-off-by: Frances Thai <31225471+notfelineit@users.noreply.github.com> * Update examples/common/scripts/vtadmin-up.sh Signed-off-by: Frances Thai <31225471+notfelineit@users.noreply.github.com> --------- Signed-off-by: notfelineit <notfelineit@gmail.com> Signed-off-by: Frances Thai <31225471+notfelineit@users.noreply.github.com> * Format vtctldclient GetKeyspace output using cli.MarshalJSON. (#12495) Signed-off-by: Matt Lord <mattalord@gmail.com> * ApplySchema: deprecate '--skip_preflight' flag (#10716) * ApplySchema: deprecate '-skip_preflight' flag Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * warn if --skip_preflight is provided Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * remove -skip_preflight from tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * MarkDeprecated, fix usage Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * remove skip_preflight test Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * streamlog: make generic (#12494) * streamlog: make generic Signed-off-by: Vicent Marti <vmg@strn.cat> * gofmt Signed-off-by: Vicent Marti <vmg@strn.cat> --------- Signed-off-by: Vicent Marti <vmg@strn.cat> * feat: copy release notes from v16 (#12516) Signed-off-by: Manan Gupta <manan@planetscale.com> * Upgrade to `go1.20.1` (#12399) * Upgrade to go1.20.1 Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Bump golangci-lint version Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Apply linter suggestions Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Signed-off-by: deepthi <deepthi@planetscale.com> * fix TestTLSClientVerifyIdentity Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Revert unwanted change in server_test Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * handle readtablet error Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * skip TestFuzz Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> * Unskip TestFuzz and fix issue Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> --------- Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr> Signed-off-by: deepthi <deepthi@planetscale.com> Co-authored-by: deepthi <deepthi@planetscale.com> * Fix `vtctldclient`'s Root command to return an error on unknown command (#12481) * Add a Run func to `vtctldclient`'s Root command to return an error on unknown command Closes #12480. Signed-off-by: Andrew Mason <andrew@planetscale.com> * Add test Signed-off-by: Andrew Mason <andrew@planetscale.com> * flags test data Signed-off-by: Andrew Mason <andrew@planetscale.com> --------- Signed-off-by: Andrew Mason <andrew@planetscale.com> * Flakes: Use new healthy shard check in vreplication e2e tests (#12502) * Use new healthy shard check in vreplication e2e tests This is needed because checking that there's a primary tablet for the shard in vtgate's healtcheck is no longer a reliable indicator that the shard has a healthy serving primary, because now a primary needs to initialize its sidecar database and wait for that to replicate via semi-sync before it becomes serving and can proceed to perform normal functions. So this delay could cause test flakiness if you required a healthy shard before continuing with the test. Signed-off-by: Matt Lord <mattalord@gmail.com> * Try to address unit test race flakes around log size They looked like this: WARNING: DATA RACE Write at 0x000005bf9b60 by goroutine 27141: github.com/spf13/pflag.newUint64Value() /home/runner/go/pkg/mod/github.com/spf13/pflag@v1.0.5/uint64.go:9 +0x5a github.com/spf13/pflag.(*FlagSet).Uint64Var() /home/runner/go/pkg/mod/github.com/spf13/pflag@v1.0.5/uint64.go:45 +0x55 vitess.io/vitess/go/vt/log.RegisterFlags() /hom…

harshit-gangal added Type: Bug Component: Query Serving Backport to: release-14.0 labels Feb 9, 2023

harshit-gangal requested review from systay, frouioui, GuptaManan100 and arthurschreiber as code owners February 9, 2023 13:39

vitess-bot bot added NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsWebsiteDocsUpdate What it says labels Feb 9, 2023

systay changed the title ~~Fix lf aggr~~ Fix aggregation on outer joins Feb 9, 2023

systay removed NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsWebsiteDocsUpdate What it says labels Feb 9, 2023

frouioui reviewed Feb 9, 2023

View reviewed changes

go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go Show resolved Hide resolved

frouioui approved these changes Feb 9, 2023

View reviewed changes

GuptaManan100 reviewed Feb 10, 2023

View reviewed changes

go/vt/vtgate/planbuilder/horizon_planning.go Show resolved Hide resolved

GuptaManan100 approved these changes Feb 10, 2023

View reviewed changes

harshit-gangal mentioned this pull request Feb 10, 2023

[16.0] fix: transaction_isolation to be applied at session level #12308

Merged

3 tasks

harshit-gangal added 3 commits February 10, 2023 18:49

fix: aggregation coming from right hand side of left join check for null

53baa2b

Signed-off-by: Harshit Gangal <harshit@planetscale.com>

test: added a sum test

017a62a

Signed-off-by: Harshit Gangal <harshit@planetscale.com>

addressed review comments

e7c44af

Signed-off-by: Harshit Gangal <harshit@planetscale.com>

harshit-gangal force-pushed the fix-lf-aggr branch from f1567c9 to e7c44af Compare February 10, 2023 13:19

harshit-gangal merged commit bc7f3ca into vitessio:main Feb 10, 2023

harshit-gangal deleted the fix-lf-aggr branch February 10, 2023 15:24

vitess-bot bot mentioned this pull request Feb 10, 2023

[release-16.0] Fix aggregation on outer joins (#12298) #12311

Merged

This was referenced Feb 10, 2023

[14.0] Fix aggregation on outer joins #12312

Merged

[15.0] Fix aggregation on outer joins (#12298) #12313

Merged

austenLacy mentioned this pull request Apr 26, 2023

Fixes the SwitchTraffic bug that wasn't respecting --dry_run for readonly and replica tablets during a resharding event Shopify/vitess#91

Merged

austenLacy mentioned this pull request Aug 1, 2023

Custom jwt http ACL policy Shopify/vitess#114

Closed

hmaurer mentioned this pull request Mar 21, 2024

oops #15542

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix aggregation on outer joins #12298

Fix aggregation on outer joins #12298

harshit-gangal commented Feb 9, 2023

vitess-bot bot commented Feb 9, 2023 •

edited by frouioui

Loading

vitess-bot bot commented Feb 10, 2023

Fix aggregation on outer joins #12298

Fix aggregation on outer joins #12298

Conversation

harshit-gangal commented Feb 9, 2023

Description

Related Issue(s)

Checklist

Deployment Notes

vitess-bot bot commented Feb 9, 2023 • edited by frouioui Loading

Review Checklist

General

If a new flag is being introduced:

If a workflow is added or modified:

Bug fixes

Non-trivial changes

New/Existing features

Backward compatibility

vitess-bot bot commented Feb 10, 2023

vitess-bot bot commented Feb 9, 2023 •

edited by frouioui

Loading