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

fix: EXPOSED-713 Allow entity batchInsert() to generate SQL for column values that match the default #2420

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Avi18971911
Copy link

Description

Summary of the change: Changed the new function for Entities under EntityClass to explicitly insert default values (when provided for columns that have default values) even when an ID is provided. This should not have any impact other than the generated SQL explicitly including columns with default values if they are specified.

Detailed description:

  • What: Entity insertion via new will generate SQL that will explicitly insert columns whose values match their default value.
  • Why: To align the DAO SQL generation with the DSL, which will explicitly insert columns whose values match their default value.
  • How: Removed an if statement that prevented explicit insertion if an ID was provided to the new() function

Type of Change

Please mark the relevant options with an "X":

  • Bug fix
  • New feature
  • Documentation update

Updates/remove existing public API methods:

  • Is breaking change

Affected databases:

  • MariaDB
  • Mysql5
  • Mysql8
  • Oracle
  • Postgres
  • SqlServer
  • H2
  • SQLite

Checklist

  • Unit tests are in place
  • The build is green (including the Detekt check)
  • All public methods affected by my PR has up to date API docs
  • Documentation for my change is up to date

Related Issues

@@ -389,14 +389,12 @@ abstract class EntityClass<ID : Any, out T : Entity<ID>>(
} finally {
entityCache.finishEntityInitialization(prototype)
}
if (entityId._value == null) {
Copy link
Author

Choose a reason for hiding this comment

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

Removed this because it seemed redundant and prevented the explicit SQL generation. If there is an ID provided, it will always write the column value to writeValues in _writeIdColumnValue_, thus the check col !in writeValues will pick it up. That being said, might be missing something here. This change doesn't affect Composite IDs because _value is correspondingly set to null after _writeIdColumnValue_

value = 94
valueWithDefault = TableWithDefaultValue.DEFAULT_VALUE
}.run {
assertTrue(this.writeValues.values.contains(TableWithDefaultValue.DEFAULT_VALUE))
Copy link
Author

Choose a reason for hiding this comment

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

This seems like it may invoke a race condition, because if inserts gets cleared before this assertion passes, I think writeValues becomes empty.

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.

1 participant