Fix NamingStrategy override of querySchema #1560
Conversation
| @@ -60,7 +64,9 @@ trait DynamicQueryDsl { | |||
| implicit def toQuoted[T <: Action[_]](q: DynamicAction[T]): Quoted[T] = q.q | |||
|
|
|||
| def dynamicQuery[T](implicit t: ClassTag[T]): DynamicEntityQuery[T] = | |||
| dynamicQuerySchema(t.runtimeClass.getName.split('.').last.split('$').last) | |||
There was a problem hiding this comment.
The dynamicQuerySchema method needs to use Fixed naming strategy since it's the equivalent of querySchema. Need to have a separate invocation of Entity here.
| @@ -58,7 +58,7 @@ class MetaDslMacro(val c: MacroContext) extends ValueComputation { | |||
| q""" | |||
| new ${c.prefix}.SchemaMeta[$t] { | |||
| private[this] val _entity = | |||
| ${c.prefix}.quote(${c.prefix}.querySchema[$t](${t.tpe.typeSymbol.name.decodedName.toString})) | |||
| ${c.prefix}.quote(${c.prefix}.impliedQuerySchema[$t](${t.tpe.typeSymbol.name.decodedName.toString})) | |||
There was a problem hiding this comment.
Need to have a separatae impliedQuerySchema method so that the parser knows to not to set the resulting Entity class's renamable field to Fixed. Basically, now all querySchema elements will have their Entity.renameable set to Fixed and impliedQuerySchema elements will have their Entity.renameable set to ByStrategy.
| @@ -13,6 +13,9 @@ private[getquill] trait QueryDsl { | |||
| @compileTimeOnly(NonQuotedException.message) | |||
| def querySchema[T](entity: String, columns: (T => (Any, String))*): EntityQuery[T] = NonQuotedException() | |||
|
|
|||
| @compileTimeOnly(NonQuotedException.message) | |||
| def impliedQuerySchema[T](entity: String, columns: (T => (Any, String))*): EntityQuery[T] = NonQuotedException() | |||
There was a problem hiding this comment.
We can't use querySchema for places where a schema needs to be implicitly inferred (e.g. insert/update/delete) because when querySchema goes into the parser, the parser thinks it's an entity with Entity.renameable = Fixed which is wrong (when insert/update/delete are used).
| @@ -104,17 +104,17 @@ trait Unliftables { | |||
| } | |||
|
|
|||
| implicit val queryUnliftable: Unliftable[Query] = Unliftable[Query] { | |||
| case q"$pack.Entity.apply(${ a: String }, ${ b: List[PropertyAlias] })" => Entity(a, b) | |||
There was a problem hiding this comment.
Not sure why the formatter decided to do this but it did
deusaquilus
force-pushed
the
rename-fix
branch
from
a539f3b
to
077a9cd
Compare
deusaquilus
force-pushed
the
rename-fix
branch
from
077a9cd
to
ac1f2a9
Compare
Fixes #766 and #1562.
Fix naming strategy override of querySchema by introducing Opinions into the AST.
An Opinion represents a piece of data that needs to be propagated through AST transformations but is not directly related to how ASTs are transformed in most stages. The Opinion,
Renameablecontrols how columns are named (i.e. whether to use aNamingStrategyor not) after most of the SQL transformations are done. Some transformations (e.g.RenamePropertieswill useOpinionsor even modify them so that the correct kind of query comes out at the end of the normalizations. That said, Opinions should be transparent in most steps of the normalization. In some cases e.g.BetaReduction, AST elements need to be neutralized (i.e. set back to defaults in the entire tree) so that this works correctly.Properties now also have an Opinion about how the
NamingStrategyaffects their name. For example something likeProperty.Opinionated(Ident(p), "s_name", Fixed)will becomep.s_nameeven if theNamingStrategyisUpperCase(whereasProperty(Ident(p), "s_name")would becomep.S_NAME). When Property is constructed withoutOpinionatedbeing used, the default opinionByStrategyis used.Entities also have the
renameableopinion. Entity has an Opinion calledrenameablewhich will be the valueFixedwhen aquerySchemais specified. That means that even if theNamingSchemaisUpperCase, the resulting query will selectt_personas opposed toT_PERSONorPerson.README.mdif applicable[WIP]to the pull request title if it's work in progresssbt scalariformFormat test:scalariformFormatto make sure that the source files are formatted@getquill/maintainers