Implement column defaults for INSERT/UPDATE #206

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?

tomchristie · 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

@@ -489,3 +531,20 @@ def __repr__(self) -> str:

    def __eq__(self, other: typing.Any) -> bool:
        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 requested 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement column defaults for INSERT/UPDATE #206

Implement column defaults for INSERT/UPDATE #206

GarbageHamburger commented May 4, 2020 •

edited

GarbageHamburger commented May 6, 2020

tomchristie commented May 7, 2020

andre-dasilva commented May 18, 2020

camgunz commented May 30, 2020

vmarkovtsev commented Jun 2, 2020 •

edited

GarbageHamburger commented Jun 18, 2020

ricosuave73 commented Jul 29, 2020

elpapi42 commented Aug 17, 2020

elpapi42 commented Aug 24, 2020

vmarkovtsev left a comment

vmarkovtsev left a comment

vmarkovtsev Aug 24, 2020

vmarkovtsev Aug 24, 2020

vmarkovtsev Aug 24, 2020

vmarkovtsev Aug 24, 2020

vmarkovtsev Aug 24, 2020

vmarkovtsev Aug 24, 2020

vmarkovtsev Aug 24, 2020

vmarkovtsev Aug 24, 2020

vmarkovtsev Aug 24, 2020

etscript commented Aug 19, 2022

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

Implement column defaults for INSERT/UPDATE #206

Are you sure you want to change the base?

Implement column defaults for INSERT/UPDATE #206

Conversation

GarbageHamburger commented May 4, 2020 • edited

GarbageHamburger commented May 6, 2020

tomchristie commented May 7, 2020

andre-dasilva commented May 18, 2020

camgunz commented May 30, 2020

vmarkovtsev commented Jun 2, 2020 • edited

GarbageHamburger commented Jun 18, 2020

ricosuave73 commented Jul 29, 2020

elpapi42 commented Aug 17, 2020

elpapi42 commented Aug 24, 2020

vmarkovtsev left a comment

Choose a reason for hiding this comment

vmarkovtsev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etscript commented Aug 19, 2022

GarbageHamburger commented May 4, 2020 •

edited

vmarkovtsev commented Jun 2, 2020 •

edited