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 upsert support #23

Merged
merged 19 commits into from
Jan 16, 2024
Merged

Add upsert support #23

merged 19 commits into from
Jan 16, 2024

Conversation

ilchuk96
Copy link
Contributor

No description provided.

@ilchuk96 ilchuk96 marked this pull request as draft January 10, 2024 17:13
@ilchuk96 ilchuk96 changed the title Draft: Add upsert support WIP: Add upsert support Jan 10, 2024
@@ -341,13 +342,16 @@ def get_bind_types(

return parameter_types

def visit_upsert(self, insert_stmt, visited_bindparam=None, **kw):
return self.visit_insert(insert_stmt, visited_bindparam, **kw).replace("INSERT", "UPSERT")
Copy link
Contributor

@LuckySting LuckySting Jan 11, 2024

Choose a reason for hiding this comment

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

issue (blocking) We should not replace all occurrences of "INSERT", because of the unexpected string values content, like:

UPSERT INTO TABLE my_table VALUES('INSERT is my favourite operation')

Also the test of this case should be

@@ -200,3 +202,49 @@ def test_select_types(self, connection):

row = connection.execute(sa.select(tb)).fetchone()
assert row == (1, "Hello World!", 3.5, True, now, today)


class TestUpsert(TablesTest):
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (blocking) Let's add also tests with several UPSERTs and UPSERT FROM SELECT, if it is supported, not sure

Copy link
Contributor

Choose a reason for hiding this comment

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

Great! Thanks

@ilchuk96 ilchuk96 marked this pull request as ready for review January 12, 2024 09:34
@ilchuk96 ilchuk96 changed the title WIP: Add upsert support Add upsert support Jan 15, 2024
@rekby rekby merged commit 6267ca9 into ydb-platform:main Jan 16, 2024
5 checks passed
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.

3 participants