GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Implement column defaults for INSERT/UPDATE by GarbageHamburger · Pull Request #206 · encode/databases

This repository was archived by the owner on Aug 19, 2025. It is now read-only.

Conversation

GarbageHamburger commented May 4, 2020 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

radriaanse, yuan1238y, fwkz, MicicFilip, zkne, istvandbsd, Tsutomu-Ikeda, randomunrandom, librehat, skuda, and 3 more reacted with thumbs up emoji

All reactions

👍 13 reactions

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

Comment thread

databases/core.py Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Remove double blank line

8803eed

Co-authored-by: Rafał Pitoń <[email protected]>

GarbageHamburger commented May 6, 2020

Copy link

Copy Markdown

Author

Any updates on this?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

lovelydinosaur commented May 7, 2020

Copy link

Copy Markdown

Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

andre-dasilva commented May 18, 2020

Copy link

Copy Markdown

@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.

yuan1238y, kellenff, sjosegarcia, MicicFilip, randomunrandom, skuda, ihor-rud, r-m-n, and oryba reacted with thumbs up emoji

All reactions

👍 9 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

camgunz commented May 30, 2020

Copy link

Copy Markdown

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

yuan1238y, dansamara, kellenff, sjosegarcia, MicicFilip, ihor-rud, r-m-n, Kim-Hyung-Jin, and oryba reacted with thumbs up emoji

All reactions

👍 9 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vmarkovtsev commented Jun 2, 2020 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

GarbageHamburger commented Jun 18, 2020

Copy link

Copy Markdown

Author

@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.

kellenff, MicicFilip, andre-dasilva, ihor-rud, r-m-n, ediskandarov-cur, oryba, antoni-szych-rtbhouse, and userino616 reacted with thumbs up emoji

All reactions

👍 9 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Merge branch 'master' into column-defaults

dansamara approved these changes

Jun 20, 2020

View reviewed changes

ricosuave73 commented Jul 29, 2020

Copy link

Copy Markdown

+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.

r-m-n reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

elpapi42 approved these changes

Aug 17, 2020

View reviewed changes

elpapi42 commented Aug 17, 2020

Copy link

Copy Markdown

This is a must, thanks

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

elpapi42 commented Aug 24, 2020

Copy link

Copy Markdown

Approve this pls

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vmarkovtsev suggested changes

Aug 24, 2020

View reviewed changes

vmarkovtsev left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

vmarkovtsev suggested changes

Aug 24, 2020

View reviewed changes

vmarkovtsev left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

These are my suggestions 👍

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

databases/core.py

Comment on lines +299 to +307


		# 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)

vmarkovtsev Aug 24, 2020

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

databases/core.py

Comment on lines +325 to +327

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

vmarkovtsev Aug 24, 2020

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

databases/core.py

		values = values or {}

		for column in query.table.c:
		if column.name in values:

vmarkovtsev Aug 24, 2020

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

databases/core.py

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

vmarkovtsev Aug 24, 2020

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

vmarkovtsev Aug 24, 2020

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

can you please group assertions together

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

databases/core.py

		continue
		else:
		value = default.arg

vmarkovtsev Aug 24, 2020

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

databases/core.py

		blows up loudly and clearly.
		"""

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

vmarkovtsev Aug 24, 2020

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We should cover this by another test which tests raising NotImplementedError

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

databases/core.py

		return str(self) == str(other)


		class FakeExecutionContext:

vmarkovtsev Aug 24, 2020

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change

	class FakeExecutionContext:
	class DummyExecutionContext:

(it's not completely fake, after all)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

databases/core.py

		"""

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

vmarkovtsev Aug 24, 2020

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ckkz-it mentioned this pull request

Nov 12, 2020

Fix sqlalchemy default values for insert and update queries #266

Closed

etscript commented Aug 19, 2022

Copy link

Copy Markdown

Any updates on this?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This branch has not been deployed

No deployments

Reviewers

4 more reviewers

rafalp

rafalp left review comments

vmarkovtsev

vmarkovtsev requested changes

dansamara

dansamara approved these changes

elpapi42

elpapi42 approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support for "default" parameter in sqlalchemy.Column

Uh oh!

There was an error while loading. Please reload this page.

10 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

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!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

10 participants

GarbageHamburger commented May 4, 2020 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

vmarkovtsev commented Jun 2, 2020 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.