Re: [PR] CAY-2842 Prevent duplicate select columns when using distinct with order by [cayenne]
stariy95 commented on code in PR #605:
URL: https://github.com/apache/cayenne/pull/605#discussion_r1487338797
##
cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingStage.java:
##
@@ -54,13 +60,15 @@ private void processOrdering(QualifierTranslator
qualifierTranslator, Translator
nodeBuilder = function("UPPER", nodeBuilder);
}
-// If query is DISTINCT then we need to add all ORDER BY clauses as
result columns
-if(!context.isDistinctSuppression()) {
-// TODO: need to check duplicates?
-// need UPPER() function here too, as some DB expect exactly the
same expression in select and in ordering
-ResultNodeDescriptor descriptor =
context.addResultNode(nodeBuilder.build().deepCopy());
-if(exp instanceof ASTAggregateFunctionCall) {
-descriptor.setAggregate(true);
+// If query is DISTINCT or GROUPING then we need to add all missing
ORDER BY clauses as result columns
Review Comment:
Maybe we could move this logic to the new stage too? Then we'll have here
just a simple ordering processing, and a new stage will process all orderings
that need to be added to the result set.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cayenne.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CAY-2842 Prevent duplicate select columns when using distinct with order by [cayenne]
Jugen commented on PR #605: URL: https://github.com/apache/cayenne/pull/605#issuecomment-1940518358 @stariy95 Have now split OrderingStage into two as suggested. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cayenne.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] CAY-2842 Prevent duplicate select columns when using distinct with order by [cayenne]
Jugen commented on code in PR #605:
URL: https://github.com/apache/cayenne/pull/605#discussion_r1486308984
##
cayenne/src/main/java/org/apache/cayenne/access/translator/select/GroupByStage.java:
##
@@ -41,7 +41,7 @@ public void perform(TranslatorContext context) {
}
}
-private boolean hasAggregate(TranslatorContext context) {
+static boolean hasAggregate(TranslatorContext context) {
Review Comment:
Moved shared logic into StageUtil instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cayenne.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CAY-2842 Prevent duplicate select columns when using distinct with order by [cayenne]
Jugen commented on code in PR #605:
URL: https://github.com/apache/cayenne/pull/605#discussion_r1486306763
##
cayenne/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java:
##
@@ -40,11 +40,11 @@ public class DefaultSelectTranslator implements
SelectTranslator {
private static final TranslationStage[] TRANSLATION_STAGES = {
new ColumnExtractorStage(),
new PrefetchNodeStage(),
-new OrderingStage(),
new QualifierTranslationStage(),
new HavingTranslationStage(),
-new GroupByStage(),
new DistinctStage(),
+new OrderingStage(),// Relies on DistinctStage
Review Comment:
I've restored the original Stage ordering in DefaultSelectTranslator and
instead moved all the shared logic code into a StageUtil class. This process
revealed that Orderings with compound paths now had to be explicitly dealt with
in OrderingStage :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cayenne.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
(cayenne) branch 5.0-FIX-CAY-2844-joint-prefetch+qualifier deleted (was 09864b79e)
This is an automated email from the ASF dual-hosted git repository. ntimofeev pushed a change to branch 5.0-FIX-CAY-2844-joint-prefetch+qualifier in repository https://gitbox.apache.org/repos/asf/cayenne.git was 09864b79e CAY-2844 Joint prefetch doesn't use ObjEntity qualifier - add failing test case This change permanently discards the following revisions: discard 09864b79e CAY-2844 Joint prefetch doesn't use ObjEntity qualifier - add failing test case
[jira] [Closed] (CAY-2839) Postgresql connection fails to escape column name if it is a reserved word, line "end".
[
https://issues.apache.org/jira/browse/CAY-2839?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Nikita Timofeev closed CAY-2839.
Resolution: Won't Fix
> Postgresql connection fails to escape column name if it is a reserved word,
> line "end".
> ---
>
> Key: CAY-2839
> URL: https://issues.apache.org/jira/browse/CAY-2839
> Project: Cayenne
> Issue Type: Bug
> Components: Core Library, Modeler
>Affects Versions: 4.2
> Environment: JDK21
>Reporter: Roland Szabó
>Priority: Major
>
> As seen in the sample stack trace, I have a database table in postgresql
> named "end". Because this is a reserved word, it should be quoted, but the
> postgresql connector does not quote column names in queries nor in Modeler,
> when I request SQL generation. The SQL generated by the Modeler can easily be
> fixed, but the queries fail.
> I have tried it with the latest postgresql library available in maven.
> {{2024-01-30 17:19:43 [apache-cayenne]
> org.apache.cayenne.log.Slf4jJdbcEventLogger logQueryError}}
> {{ INFO: *** error.}}
> {{ org.postgresql.util.PSQLException: ERROR: syntax error at or near "end"}}
> {{ Position: 26}}
> {{ at
> org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2676)}}
> {{ at
> org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:2366)}}
> {{ at
> org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:356)}}
> {{ at
> org.postgresql.jdbc.PgStatement.executeInternal(PgStatement.java:496)}}
> {{ at org.postgresql.jdbc.PgStatement.execute(PgStatement.java:413)}}
> {{ at
> org.postgresql.jdbc.PgPreparedStatement.executeWithFlags(PgPreparedStatement.java:190)}}
> {{ at
> org.postgresql.jdbc.PgPreparedStatement.executeUpdate(PgPreparedStatement.java:152)}}
> {{ at
> org.apache.cayenne.access.jdbc.BatchAction.runAsIndividualQueries(BatchAction.java:196)}}
> {{ at
> org.apache.cayenne.access.jdbc.BatchAction.performAction(BatchAction.java:93)}}
> {{ at
> org.apache.cayenne.access.DataNodeQueryAction.runQuery(DataNodeQueryAction.java:97)}}
> {{ at
> org.apache.cayenne.access.DataNode.performQueries(DataNode.java:273)}}
> {{ at
> org.apache.cayenne.access.flush.DefaultDataDomainFlushAction.lambda$executeQueries$6(DefaultDataDomainFlushAction.java:178)}}
> {{ at java.base/java.util.HashMap.forEach(HashMap.java:1429)}}
> {{ at
> org.apache.cayenne.access.flush.DefaultDataDomainFlushAction.executeQueries(DefaultDataDomainFlushAction.java:177)}}
> {{ at
> org.apache.cayenne.access.flush.DefaultDataDomainFlushAction.flush(DefaultDataDomainFlushAction.java:90)}}
> {{ at
> org.apache.cayenne.access.DataDomain.onSyncFlush(DataDomain.java:637)}}
> {{ at
> org.apache.cayenne.access.DataDomain.onSyncNoFilters(DataDomain.java:609)}}
> {{ at
> org.apache.cayenne.access.DataDomain$DataDomainSyncFilterChain.onSync(DataDomain.java:835)}}
> {{ at
> org.apache.cayenne.tx.TransactionFilter.lambda$onSync$0(TransactionFilter.java:61)}}
> {{ at
> org.apache.cayenne.tx.DefaultTransactionManager$BaseTransactionHandler.performInTransaction(DefaultTransactionManager.java:180)}}
> {{ at
> org.apache.cayenne.tx.DefaultTransactionManager$NestedTransactionHandler.handle(DefaultTransactionManager.java:93)}}
> {{ at
> org.apache.cayenne.tx.DefaultTransactionManager.performInTransaction(DefaultTransactionManager.java:62)}}
> {{ at
> org.apache.cayenne.tx.DefaultTransactionManager.performInTransaction(DefaultTransactionManager.java:40)}}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
(cayenne) 01/01: CAY-2844 Joint prefetch doesn't use ObjEntity qualifier - add failing test case
This is an automated email from the ASF dual-hosted git repository.
ntimofeev pushed a commit to branch 5.0-FIX-CAY-2844-joint-prefetch+qualifier
in repository https://gitbox.apache.org/repos/asf/cayenne.git
commit 09864b79ec297ab07be34990d22f8af20714a361
Author: Nikita Timofeev
AuthorDate: Mon Feb 12 16:11:12 2024 +0400
CAY-2844 Joint prefetch doesn't use ObjEntity qualifier
- add failing test case
---
.../org/apache/cayenne/CDOQualifiedEntitiesIT.java | 72 ++
1 file changed, 72 insertions(+)
diff --git
a/cayenne/src/test/java/org/apache/cayenne/CDOQualifiedEntitiesIT.java
b/cayenne/src/test/java/org/apache/cayenne/CDOQualifiedEntitiesIT.java
index e5bffea2d..68b722774 100644
--- a/cayenne/src/test/java/org/apache/cayenne/CDOQualifiedEntitiesIT.java
+++ b/cayenne/src/test/java/org/apache/cayenne/CDOQualifiedEntitiesIT.java
@@ -128,6 +128,78 @@ public class CDOQualifiedEntitiesIT extends RuntimeCase {
}
}
+@Test
+public void testJointPrefetchToMany() throws Exception {
+if (accessStackAdapter.supportsNullBoolean()) {
+
+createReadToManyDataSet();
+
+List roots = ObjectSelect.query(Qualified1.class)
+.prefetch(Qualified1.QUALIFIED2S.joint())
+.select(context);
+
+assertEquals(1, roots.size());
+
+Qualified1 root = roots.get(0);
+
+assertEquals("OX1", root.getName());
+
+List related = root.getQualified2s();
+assertEquals(1, related.size());
+
+Qualified2 r = related.get(0);
+assertEquals("OY1", r.getName());
+}
+}
+
+@Test
+public void testDisjointPrefetchToMany() throws Exception {
+if (accessStackAdapter.supportsNullBoolean()) {
+
+createReadToManyDataSet();
+
+List roots = ObjectSelect.query(Qualified1.class)
+.prefetch(Qualified1.QUALIFIED2S.disjoint())
+.select(context);
+
+assertEquals(1, roots.size());
+
+Qualified1 root = roots.get(0);
+
+assertEquals("OX1", root.getName());
+
+List related = root.getQualified2s();
+assertEquals(1, related.size());
+
+Qualified2 r= related.get(0);
+assertEquals("OY1", r.getName());
+}
+}
+
+@Test
+public void testDisjointByIdPrefetchToMany() throws Exception {
+if (accessStackAdapter.supportsNullBoolean()) {
+
+createReadToManyDataSet();
+
+List roots = ObjectSelect.query(Qualified1.class)
+.prefetch(Qualified1.QUALIFIED2S.disjointById())
+.select(context);
+
+assertEquals(1, roots.size());
+
+Qualified1 root = roots.get(0);
+
+assertEquals("OX1", root.getName());
+
+List related = root.getQualified2s();
+assertEquals(1, related.size());
+
+Qualified2 r = related.get(0);
+assertEquals("OY1", r.getName());
+}
+}
+
@Test
public void testReadToOne() throws Exception {
if (accessStackAdapter.supportsNullBoolean()) {
(cayenne) branch 5.0-FIX-CAY-2844-joint-prefetch+qualifier created (now 09864b79e)
This is an automated email from the ASF dual-hosted git repository. ntimofeev pushed a change to branch 5.0-FIX-CAY-2844-joint-prefetch+qualifier in repository https://gitbox.apache.org/repos/asf/cayenne.git at 09864b79e CAY-2844 Joint prefetch doesn't use ObjEntity qualifier - add failing test case This branch includes the following new commits: new 09864b79e CAY-2844 Joint prefetch doesn't use ObjEntity qualifier - add failing test case The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference.
Re: [PR] CAY-2842 Prevent duplicate select columns when using distinct with order by [cayenne]
stariy95 commented on code in PR #605:
URL: https://github.com/apache/cayenne/pull/605#discussion_r1486041480
##
cayenne/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java:
##
@@ -40,11 +40,11 @@ public class DefaultSelectTranslator implements
SelectTranslator {
private static final TranslationStage[] TRANSLATION_STAGES = {
new ColumnExtractorStage(),
new PrefetchNodeStage(),
-new OrderingStage(),
new QualifierTranslationStage(),
new HavingTranslationStage(),
-new GroupByStage(),
new DistinctStage(),
+new OrderingStage(),// Relies on DistinctStage
Review Comment:
Translation stages are there just to split all the complext logic to
apprehensible chunks. So it should be totally fine to add more stages. This
logic is separated enough, we'll only need a way to read ordering nodes in a
later stage:
```java
if (isDistinctOrGroupByQuery) {
Optional column = getResultColumn(context,
order(nodeBuilder));
if (!column.isPresent()) {
// deepCopy as some DB expect exactly the same expression in
select and in ordering
ResultNodeDescriptor descriptor =
context.addResultNode(nodeBuilder.build().deepCopy());
if(exp instanceof ASTAggregateFunctionCall) {
descriptor.setAggregate(true);
}
}
}
```
##
cayenne/src/main/java/org/apache/cayenne/access/translator/select/GroupByStage.java:
##
@@ -41,7 +41,7 @@ public void perform(TranslatorContext context) {
}
}
-private boolean hasAggregate(TranslatorContext context) {
+static boolean hasAggregate(TranslatorContext context) {
Review Comment:
Yeah, we may add a flag to the context and keep the logic in the
`GroupByStage`, then it should be decent enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cayenne.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CAY-2842 Prevent duplicate select columns when using distinct with order by [cayenne]
Jugen commented on code in PR #605:
URL: https://github.com/apache/cayenne/pull/605#discussion_r1486015595
##
cayenne/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java:
##
@@ -40,11 +40,11 @@ public class DefaultSelectTranslator implements
SelectTranslator {
private static final TranslationStage[] TRANSLATION_STAGES = {
new ColumnExtractorStage(),
new PrefetchNodeStage(),
-new OrderingStage(),
new QualifierTranslationStage(),
new HavingTranslationStage(),
-new GroupByStage(),
new DistinctStage(),
+new OrderingStage(),// Relies on DistinctStage
Review Comment:
Or combine this with `hasAggregate` from GroupByStage into a `StageUtil`
class ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cayenne.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CAY-2842 Prevent duplicate select columns when using distinct with order by [cayenne]
Jugen commented on code in PR #605:
URL: https://github.com/apache/cayenne/pull/605#discussion_r1486012235
##
cayenne/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java:
##
@@ -40,11 +40,11 @@ public class DefaultSelectTranslator implements
SelectTranslator {
private static final TranslationStage[] TRANSLATION_STAGES = {
new ColumnExtractorStage(),
new PrefetchNodeStage(),
-new OrderingStage(),
new QualifierTranslationStage(),
new HavingTranslationStage(),
-new GroupByStage(),
new DistinctStage(),
+new OrderingStage(),// Relies on DistinctStage
Review Comment:
Actually it may be better moving it into a separate `DistinctUtil` class ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cayenne.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CAY-2842 Prevent duplicate select columns when using distinct with order by [cayenne]
Jugen commented on code in PR #605:
URL: https://github.com/apache/cayenne/pull/605#discussion_r1485996375
##
cayenne/src/main/java/org/apache/cayenne/access/translator/select/DefaultSelectTranslator.java:
##
@@ -40,11 +40,11 @@ public class DefaultSelectTranslator implements
SelectTranslator {
private static final TranslationStage[] TRANSLATION_STAGES = {
new ColumnExtractorStage(),
new PrefetchNodeStage(),
-new OrderingStage(),
new QualifierTranslationStage(),
new HavingTranslationStage(),
-new GroupByStage(),
new DistinctStage(),
+new OrderingStage(),// Relies on DistinctStage
Review Comment:
Another possible solution is to move the bulk of the logic in
`DistinctStage.perform` to `TranslatorContext`, then I can revert this change.
What do you think ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cayenne.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CAY-2842 Prevent duplicate select columns when using distinct with order by [cayenne]
Jugen commented on code in PR #605:
URL: https://github.com/apache/cayenne/pull/605#discussion_r1485966341
##
cayenne/src/main/java/org/apache/cayenne/access/translator/select/GroupByStage.java:
##
@@ -41,7 +41,7 @@ public void perform(TranslatorContext context) {
}
}
-private boolean hasAggregate(TranslatorContext context) {
+static boolean hasAggregate(TranslatorContext context) {
Review Comment:
A possibility is moving it to TranslatorContext, what do you think ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cayenne.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CAY-2842 Prevent duplicate select columns when using distinct with order by [cayenne]
Jugen commented on code in PR #605:
URL: https://github.com/apache/cayenne/pull/605#discussion_r1485959529
##
cayenne/src/main/java/org/apache/cayenne/access/translator/select/OrderingStage.java:
##
@@ -71,4 +85,26 @@ private void processOrdering(QualifierTranslator
qualifierTranslator, Translator
context.getSelectBuilder().orderBy(orderingNodeBuilder);
}
+private Optional getResultColumn(TranslatorContext context,
OrderingNodeBuilder orderBuilder) {
+String orderStr = getSqlString(orderBuilder);
+
+return context.getResultNodeList().stream()
+.map(result -> result.getNode())
+// the column may have an alias so just check that the
orderStr part matches
+.filter(resultNode ->
getSqlString(node(resultNode)).startsWith(orderStr))
+.findFirst();
Review Comment:
I thought it's a bit heavy as well. Will need guidance later .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cayenne.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CAY-2842 Prevent duplicate select columns when using distinct with order by [cayenne]
Jugen commented on code in PR #605:
URL: https://github.com/apache/cayenne/pull/605#discussion_r1485952787
##
cayenne/src/main/java/org/apache/cayenne/access/translator/select/DistinctStage.java:
##
@@ -70,7 +70,7 @@ public void perform(TranslatorContext context) {
context.getSelectBuilder().distinct();
}
-private boolean hasToManyJoin(TranslatorContext context) {
+static boolean hasToManyJoin(TranslatorContext context) {
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cayenne.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] CAY-2842 Prevent duplicate select columns when using distinct with order by [cayenne]
stariy95 commented on PR #605: URL: https://github.com/apache/cayenne/pull/605#issuecomment-1938319464 @Jugen Thanks for keeping it up. I did my review, but it seems a bit early :) There are mostly minor things with the exception of the translation stages rearrangement. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cayenne.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
