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

Add Partitioned DML #94

Merged
merged 3 commits into from
Oct 17, 2020
Merged

Add Partitioned DML #94

merged 3 commits into from
Oct 17, 2020

Conversation

yfuruyama
Copy link
Collaborator

Syntax:

PARTITIONED (UPDATE|DELETE) ...;

Example:

spanner> select * from Songs;
+----------+---------+---------+-------------------------+----------+-----------+
| SingerId | AlbumId | TrackId | SongName                | Duration | SongGenre |
+----------+---------+---------+-------------------------+----------+-----------+
| 2        | 1       | 1       | Let's Get Back Together | 182      | COUNTRY   |
| 2        | 1       | 2       | Starting Again          | 156      | ROCK      |
| 2        | 1       | 3       | I Knew You Were Magic   | 294      | BLUES     |
| 2        | 1       | 4       | 42                      | 185      | CLASSICAL |
| 2        | 1       | 5       | Blue                    | 238      | BLUES     |
| 2        | 1       | 6       | Nothing Is The Same     | 303      | BLUES     |
| 2        | 1       | 7       | The Second Time         | 255      | ROCK      |
| 2        | 3       | 1       | Fight Story             | 194      | ROCK      |
| 3        | 1       | 1       | Not About The Guitar    | 278      | BLUES     |
+----------+---------+---------+-------------------------+----------+-----------+
9 rows in set (5.48 msecs)

spanner> partitioned update Songs set SongName = "Foo Bar" WHERE SongGenre = 'ROCK';
Query OK, 3 rows affected (3.75 sec)

spanner> select * from Songs;
+----------+---------+---------+-------------------------+----------+-----------+
| SingerId | AlbumId | TrackId | SongName                | Duration | SongGenre |
+----------+---------+---------+-------------------------+----------+-----------+
| 2        | 1       | 1       | Let's Get Back Together | 182      | COUNTRY   |
| 2        | 1       | 2       | Foo Bar                 | 156      | ROCK      |
| 2        | 1       | 3       | I Knew You Were Magic   | 294      | BLUES     |
| 2        | 1       | 4       | 42                      | 185      | CLASSICAL |
| 2        | 1       | 5       | Blue                    | 238      | BLUES     |
| 2        | 1       | 6       | Nothing Is The Same     | 303      | BLUES     |
| 2        | 1       | 7       | Foo Bar                 | 255      | ROCK      |
| 2        | 3       | 1       | Foo Bar                 | 194      | ROCK      |
| 3        | 1       | 1       | Not About The Guitar    | 278      | BLUES     |
+----------+---------+---------+-------------------------+----------+-----------+
9 rows in set (51.09 msecs)

spanner> partitioned delete from Songs WHERE SongGenre = 'ROCK';
Query OK, 3 rows affected (3.30 sec)

spanner> select * from Songs;
+----------+---------+---------+-------------------------+----------+-----------+
| SingerId | AlbumId | TrackId | SongName                | Duration | SongGenre |
+----------+---------+---------+-------------------------+----------+-----------+
| 2        | 1       | 1       | Let's Get Back Together | 182      | COUNTRY   |
| 2        | 1       | 3       | I Knew You Were Magic   | 294      | BLUES     |
| 2        | 1       | 4       | 42                      | 185      | CLASSICAL |
| 2        | 1       | 5       | Blue                    | 238      | BLUES     |
| 2        | 1       | 6       | Nothing Is The Same     | 303      | BLUES     |
| 3        | 1       | 1       | Not About The Guitar    | 278      | BLUES     |
+----------+---------+---------+-------------------------+----------+-----------+
6 rows in set (6.84 msecs)

close: #93

@google-cla google-cla bot added the cla: yes CLA signed label Oct 17, 2020
@yfuruyama yfuruyama requested a review from apstndb October 17, 2020 12:19
@yfuruyama
Copy link
Collaborator Author

@apstndb Please review this PR when you have time. Thanks!

Copy link
Collaborator

@apstndb apstndb left a comment

Choose a reason for hiding this comment

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

It is a major update!

TRUNCATE TABLE can be an alias of PARTITIONED DELETE `tbl` WHERE TRUE in both of semantics and implementation.
The behavior will be clear, so why not make it an alias by the time you release it?

@@ -78,6 +78,11 @@ var (
// DML
dmlRe = regexp.MustCompile(`(?is)^(INSERT|UPDATE|DELETE)\s+.+$`)

// Partitioned DML
// In fact, INSERT is not supported in a Partitioned DML, but accept it for showing better error message.
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

statement.go Outdated Show resolved Hide resolved
@yfuruyama
Copy link
Collaborator Author

TRUNCATE TABLE can be an alias of PARTITIONED DELETE tbl WHERE TRUE in both of semantics and implementation.
The behavior will be clear, so why not make it an alias by the time you release it?

Initially I implemented so, but I have found error messages (example) have to be different from Partitioned DML, so I kept the implementation of TRUNCATE TABLE as it is. Using Partitioned DML for TRUNCATE TABLE is part of implementation details and I think it should be hidden from users.

@apstndb
Copy link
Collaborator

apstndb commented Oct 17, 2020

OK, I understand the reason.
I agree it is an implementation detail.

Another choice:

  • It is message only reason so switch message (e.g. by enum value in PartitionedDmlStatement)

Copy link
Collaborator

@apstndb apstndb left a comment

Choose a reason for hiding this comment

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

LGTM

@yfuruyama
Copy link
Collaborator Author

Thanks for reviewing!

It is message only reason so switch message (e.g. by enum value in PartitionedDmlStatement)

Yeah that's totally fine. Then let's change the implementation once we emulate other SQL functionalities with Partitioned DML 👍

@yfuruyama yfuruyama merged commit 8f5b2a6 into master Oct 17, 2020
@yfuruyama yfuruyama deleted the feature/pdml branch October 17, 2020 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes CLA signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Partitioned DML
2 participants