-
Notifications
You must be signed in to change notification settings - Fork 24
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/ctas oracle column definitions #237
base: main
Are you sure you want to change the base?
Conversation
morf-oracle/src/main/java/org/alfasoftware/morf/jdbc/oracle/OracleDialect.java
Outdated
Show resolved
Hide resolved
morf-oracle/src/main/java/org/alfasoftware/morf/jdbc/oracle/OracleDialect.java
Outdated
Show resolved
Hide resolved
Kudos, SonarCloud Quality Gate passed! |
Alternative to #241 |
this.fields.clear(); | ||
Iterables.addAll(this.fields, Builder.Helper.buildAll(fields)); | ||
return castToChild(this); | ||
} |
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.
Rename to replaceFields()
} else { | ||
fieldsToInclude.add(field); | ||
} | ||
} |
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.
if (field instanceof NullFieldLiteral || field instanceof FieldReference)
I think we want to handle all fields, including expressions, etc.
So, basically, there should be no else
in there.
column("nullField", DataType.STRING, 3) | ||
).indexes( | ||
index("SomeTable_1").columns("otherField") | ||
); |
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.
Notice that this index is not present in the resulting SQL (which is correct and by design).
Indexes cannot be part of a CTAS, which is documented at SchemaEditor.addTableFrom(Table, SelectStatement).
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.
Was this not possible to add as tests for all dialects?
Any advantage to have this as a separate test class?
Fixed a bug with CTAS, Oracle Dialect will now handle null values when creating a new table as select.