Implement column defaults for INSERT/UPDATE by GarbageHamburger · Pull Request #206 · encode/databases

GarbageHamburger · 2020-05-04T16:36:58Z

This PR implements usage of column defaults from SQLAlchemy. In regular SQLAlchemy the default column values are fetched using ExecutionContext._exec_default. However, we don't use the ExecutionContext here, so a function is implemented to take its place for column defaults.

Fixes #72

MySQL rounds datetimes to seconds.

databases/core.py

Co-authored-by: Rafał Pitoń <kontakt@rpiton.com>

GarbageHamburger · 2020-05-06T18:26:24Z

Any updates on this?

lovelydinosaur · 2020-05-07T12:26:36Z

It's difficult because I'm not really sure if we want this functionality or not.

andre-dasilva · 2020-05-18T09:14:15Z

@tomchristie I am just curious. Why would you not want this functionality? I am currently writing a application with fastapi and encode/databases and i want the timestamp to be created on insert. currently i don't have a way of doing this.

camgunz · 2020-05-30T18:45:01Z

It also seems necessary for UUID primary keys, which are pretty common these days.

vmarkovtsev · 2020-06-02T06:35:27Z

The column defaults are higher level sqla API ("Query") than the API used in databases ("Core"). Emulating them properly is quite hard. For example, the functions can access the context in several ways, and filling it is twisted.

The problem can be solved outside of databases package by adding a new method to your model base class to explicitly create defaults.

GarbageHamburger · 2020-06-18T12:54:26Z

@vmarkovtsev SQLAlchemy core does have defaults, see the default parameter for Column.__init__. I agree that correctly emulating defaults is hard due to the types of things a column default can be (it can be a ClauseElement!), but that's no reason for it to not be included.

ricosuave73 · 2020-07-29T13:40:58Z

+1 for this. I followed the FastAPI tutorial and tried to implement a UUID for sqlite. I couldn't figure out why it didn't work until I specifically checked this repo for issues around the defaults.
And defaults are indeed part of sqlalchemy core.

elpapi42 · 2020-08-17T22:17:59Z

This is a must, thanks

elpapi42 · 2020-08-24T20:44:52Z

Approve this pls

vmarkovtsev

Sorry fellows, I wish I approved this, but this looks too hacky right now to be included as a general solution.

vmarkovtsev

These are my suggestions 👍

vmarkovtsev · 2020-08-24T22:36:26Z

databases/core.py

+
+        # 2 paths where we apply column defaults:
+        # - values are supplied (the object must be a ValuesBase)
+        # - values is None but the object is a ValuesBase
+        if values is not None and not isinstance(query, ValuesBase):
+            raise TypeError("values supplied but query doesn't support .values()")
+
+        if values is not None or isinstance(query, ValuesBase):
+            values = Connection._apply_column_defaults(query, values)


Suggested change

# 2 paths where we apply column defaults:

# - values are supplied (the object must be a ValuesBase)

# - values is None but the object is a ValuesBase

if values is not None and not isinstance(query, ValuesBase):

raise TypeError("values supplied but query doesn't support .values()")

if values is not None or isinstance(query, ValuesBase):

values = Connection._apply_column_defaults(query, values)

elif values:

assert isinstance(query, ValuesBase)

values = self._apply_column_defaults(query, values)

vmarkovtsev · 2020-08-24T22:37:30Z

databases/core.py

+                if default.is_sequence:  # pragma: no cover
+                    # TODO: support sequences
+                    continue


assert not default.is_sequence, "sequences are not supported, PRs welcome"

vmarkovtsev · 2020-08-24T22:38:28Z

databases/core.py

+        values = values or {}
+
+        for column in query.table.c:
+            if column.name in values:


values can have columns as keys, in such cases this will be always false

vmarkovtsev · 2020-08-24T22:39:22Z

databases/core.py

+                    continue
+                elif default.is_callable:
+                    value = default.arg(FakeExecutionContext())
+                elif default.is_clause_element:  # pragma: no cover


Suggested change

elif default.is_clause_element: # pragma: no cover

assert not default.is_clause_element, "clause defaults are not supported, PRs welcome"

can you please group assertions together

vmarkovtsev · 2020-08-24T22:40:15Z

databases/core.py

+                    continue
+                else:
+                    value = default.arg
+


Suggested change

vmarkovtsev · 2020-08-24T22:41:21Z

databases/core.py

+    blows up loudly and clearly.
+    """
+
+    def __getattr__(self, _: str) -> typing.NoReturn:  # pragma: no cover


We should cover this by another test which tests raising NotImplementedError

vmarkovtsev · 2020-08-24T22:42:00Z

databases/core.py

        return str(self) == str(other)
+
+
+class FakeExecutionContext:


Suggested change

class FakeExecutionContext:

class DummyExecutionContext:

(it's not completely fake, after all)

vmarkovtsev · 2020-08-24T22:43:40Z

databases/core.py

+    """
+
+    def __getattr__(self, _: str) -> typing.NoReturn:  # pragma: no cover
+        raise NotImplementedError(


btw: my own custom implementation of this (yep, I have a similar hack in my prod), I pass the current values to this context so that it becomes much more useful.

etscript · 2022-08-19T08:41:32Z

Any updates on this?

GarbageHamburger added 2 commits May 4, 2020 16:34

Implement column defaults for INSERT/UPDATE

dcf3378

Fix column defaults test for MySQL

f66013e

MySQL rounds datetimes to seconds.

GarbageHamburger force-pushed the column-defaults branch from 9d9080a to f66013e Compare May 4, 2020 17:01

rafalp reviewed May 4, 2020

View reviewed changes

databases/core.py Show resolved Hide resolved

Remove double blank line

8803eed

Co-authored-by: Rafał Pitoń <kontakt@rpiton.com>

Merge branch 'master' into column-defaults

8148350

dansamara approved these changes Jun 20, 2020

View reviewed changes

elpapi42 approved these changes Aug 17, 2020

View reviewed changes

vmarkovtsev suggested changes Aug 24, 2020

View reviewed changes

ckkz-it mentioned this pull request Nov 12, 2020

Fix sqlalchemy default values for insert and update queries #266

Closed

	elif default.is_clause_element: # pragma: no cover
	assert not default.is_clause_element, "clause defaults are not supported, PRs welcome"

Conversation

GarbageHamburger commented May 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

GarbageHamburger commented May 6, 2020

Uh oh!

lovelydinosaur commented May 7, 2020

Uh oh!

andre-dasilva commented May 18, 2020

Uh oh!

camgunz commented May 30, 2020

Uh oh!

vmarkovtsev commented Jun 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GarbageHamburger commented Jun 18, 2020

Uh oh!

ricosuave73 commented Jul 29, 2020

Uh oh!

elpapi42 commented Aug 17, 2020

Uh oh!

elpapi42 commented Aug 24, 2020

Uh oh!

vmarkovtsev left a comment

Choose a reason for hiding this comment

Uh oh!

vmarkovtsev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etscript commented Aug 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

GarbageHamburger commented May 4, 2020 •

edited

Loading

vmarkovtsev commented Jun 2, 2020 •

edited

Loading