You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openjpa.apache.org by rz...@apache.org on 2026/09/02 17:16:24 UTC
(openjpa) branch master updated: [OPENJPA-2983] Resolve EXTRACT temporal fields by constant, not by name (#166)
This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/openjpa.git
The following commit(s) were added to refs/heads/master by this push:
new 695fd679f [OPENJPA-2983] Resolve EXTRACT temporal fields by constant, not by name (#166)
695fd679f is described below
commit 695fd679f2555a8e9ad05884de1662f360c4439a
Author: Richard Zowalla <13...@users.noreply.github.com>
AuthorDate: Wed Sep 2 19:16:19 2026 +0200
[OPENJPA-2983] Resolve EXTRACT temporal fields by constant, not by name (#166)
CriteriaBuilderImpl.extract() derived the DateTimeExtractField from
field.toString().toUpperCase(), which depends on the default locale and on
the textual representation the specification constants happen to use. Map
the LocalDateField / LocalTimeField / LocalDateTimeField constants to their
DateTimeExtractField equivalents by identity instead, and reject any other
temporal field with a meaningful message instead of letting Enum.valueOf
fail.
TestExtractTemporalField pins every entry of the new mapping table, which
is where the risk now sits: it is a characterisation test of the 16 hand
written pairs, not a reproduction of the original defect, since the name
based lookup produced the same result for all constants once Locale.ROOT
was passed.
---
.../persistence/criteria/CriteriaBuilderImpl.java | 42 ++++++++-
.../openjpa/persistence/criteria/Expressions.java | 4 +
.../criteria/TestExtractTemporalField.java | 100 +++++++++++++++++++++
3 files changed, 142 insertions(+), 4 deletions(-)
diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaBuilderImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaBuilderImpl.java
index 2200b10a8..864bce194 100644
--- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaBuilderImpl.java
+++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/CriteriaBuilderImpl.java
@@ -29,8 +29,8 @@ import java.time.LocalTime;
import java.time.temporal.Temporal;
import java.util.Collection;
import java.util.Collections;
+import java.util.IdentityHashMap;
import java.util.List;
-import java.util.Locale;
import java.util.Map;
import java.util.Set;
@@ -44,6 +44,9 @@ import jakarta.persistence.criteria.Expression;
import jakarta.persistence.criteria.From;
import jakarta.persistence.criteria.Join;
import jakarta.persistence.criteria.ListJoin;
+import jakarta.persistence.criteria.LocalDateField;
+import jakarta.persistence.criteria.LocalDateTimeField;
+import jakarta.persistence.criteria.LocalTimeField;
import jakarta.persistence.criteria.MapJoin;
import jakarta.persistence.criteria.Nulls;
import jakarta.persistence.criteria.Order;
@@ -83,6 +86,34 @@ import org.apache.openjpa.persistence.meta.Types;
*/
public class CriteriaBuilderImpl implements OpenJPACriteriaBuilder, ExpressionParser {
private static final long serialVersionUID = 1L;
+
+ /**
+ * The temporal fields EXTRACT accepts, keyed by the constants defined by the specification.
+ * These constants are not enums, but they are the only instances their classes hand out, so
+ * they are matched by identity rather than by their (locale sensitive, lower case) name.
+ */
+ private static final Map<TemporalField<?, ?>, DateTimeExtractField> EXTRACT_FIELDS;
+ static {
+ Map<TemporalField<?, ?>, DateTimeExtractField> fields = new IdentityHashMap<>();
+ fields.put(LocalDateField.YEAR, DateTimeExtractField.YEAR);
+ fields.put(LocalDateField.QUARTER, DateTimeExtractField.QUARTER);
+ fields.put(LocalDateField.MONTH, DateTimeExtractField.MONTH);
+ fields.put(LocalDateField.WEEK, DateTimeExtractField.WEEK);
+ fields.put(LocalDateField.DAY, DateTimeExtractField.DAY);
+ fields.put(LocalTimeField.HOUR, DateTimeExtractField.HOUR);
+ fields.put(LocalTimeField.MINUTE, DateTimeExtractField.MINUTE);
+ fields.put(LocalTimeField.SECOND, DateTimeExtractField.SECOND);
+ fields.put(LocalDateTimeField.YEAR, DateTimeExtractField.YEAR);
+ fields.put(LocalDateTimeField.QUARTER, DateTimeExtractField.QUARTER);
+ fields.put(LocalDateTimeField.MONTH, DateTimeExtractField.MONTH);
+ fields.put(LocalDateTimeField.WEEK, DateTimeExtractField.WEEK);
+ fields.put(LocalDateTimeField.DAY, DateTimeExtractField.DAY);
+ fields.put(LocalDateTimeField.HOUR, DateTimeExtractField.HOUR);
+ fields.put(LocalDateTimeField.MINUTE, DateTimeExtractField.MINUTE);
+ fields.put(LocalDateTimeField.SECOND, DateTimeExtractField.SECOND);
+ EXTRACT_FIELDS = Collections.unmodifiableMap(fields);
+ }
+
private MetamodelImpl _model;
public OpenJPACriteriaBuilder setMetaModel(MetamodelImpl model) {
@@ -1202,10 +1233,13 @@ public class CriteriaBuilderImpl implements OpenJPACriteriaBuilder, ExpressionPa
@Override
public <N, T extends Temporal> Expression<N> extract(TemporalField<N, T> field, Expression<T> temporal) {
- String fieldName = field.toString().toUpperCase(Locale.ROOT);
- DateTimeExtractField extractField = DateTimeExtractField.valueOf(fieldName);
+ DateTimeExtractField extractField = EXTRACT_FIELDS.get(field);
+ if (extractField == null) {
+ throw new IllegalArgumentException("Unsupported temporal field for EXTRACT: " + field);
+ }
@SuppressWarnings("unchecked")
- Class<N> resultType = fieldName.equals("SECOND") ? (Class<N>) Double.class : (Class<N>) Integer.class;
+ Class<N> resultType = (Class<N>) (extractField == DateTimeExtractField.SECOND
+ ? Double.class : Integer.class);
return new Expressions.ExtractField<>(resultType, extractField, temporal);
}
diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/Expressions.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/Expressions.java
index 02d247f26..4d03e40d8 100644
--- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/Expressions.java
+++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/Expressions.java
@@ -414,6 +414,10 @@ class Expressions {
this.field = field;
}
+ public DateTimeExtractField getField() {
+ return field;
+ }
+
@Override
public Value toValue(ExpressionFactory factory, CriteriaQueryImpl<?> q) {
Value value = factory.getDateTimeField(field, Expressions.toValue(e, factory, q));
diff --git a/openjpa-persistence/src/test/java/org/apache/openjpa/persistence/criteria/TestExtractTemporalField.java b/openjpa-persistence/src/test/java/org/apache/openjpa/persistence/criteria/TestExtractTemporalField.java
new file mode 100644
index 000000000..815a3055c
--- /dev/null
+++ b/openjpa-persistence/src/test/java/org/apache/openjpa/persistence/criteria/TestExtractTemporalField.java
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.openjpa.persistence.criteria;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThrows;
+
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.temporal.Temporal;
+
+import jakarta.persistence.criteria.Expression;
+import jakarta.persistence.criteria.LocalDateField;
+import jakarta.persistence.criteria.LocalDateTimeField;
+import jakarta.persistence.criteria.LocalTimeField;
+import jakarta.persistence.criteria.TemporalField;
+
+import org.apache.openjpa.kernel.exps.DateTimeExtractField;
+import org.junit.Test;
+
+/**
+ * Tests that EXTRACT resolves each temporal field constant of the specification to its
+ * kernel equivalent, rather than deriving it from the constant's textual representation.
+ */
+public class TestExtractTemporalField {
+
+ private final CriteriaBuilderImpl cb = new CriteriaBuilderImpl();
+
+ @Test
+ public void testLocalDateFields() {
+ assertExtracts(DateTimeExtractField.YEAR, LocalDateField.YEAR, LocalDate.class);
+ assertExtracts(DateTimeExtractField.QUARTER, LocalDateField.QUARTER, LocalDate.class);
+ assertExtracts(DateTimeExtractField.MONTH, LocalDateField.MONTH, LocalDate.class);
+ assertExtracts(DateTimeExtractField.WEEK, LocalDateField.WEEK, LocalDate.class);
+ assertExtracts(DateTimeExtractField.DAY, LocalDateField.DAY, LocalDate.class);
+ }
+
+ @Test
+ public void testLocalTimeFields() {
+ assertExtracts(DateTimeExtractField.HOUR, LocalTimeField.HOUR, LocalTime.class);
+ assertExtracts(DateTimeExtractField.MINUTE, LocalTimeField.MINUTE, LocalTime.class);
+ assertExtracts(DateTimeExtractField.SECOND, LocalTimeField.SECOND, LocalTime.class);
+ }
+
+ @Test
+ public void testLocalDateTimeFields() {
+ assertExtracts(DateTimeExtractField.YEAR, LocalDateTimeField.YEAR, LocalDateTime.class);
+ assertExtracts(DateTimeExtractField.QUARTER, LocalDateTimeField.QUARTER, LocalDateTime.class);
+ assertExtracts(DateTimeExtractField.MONTH, LocalDateTimeField.MONTH, LocalDateTime.class);
+ assertExtracts(DateTimeExtractField.WEEK, LocalDateTimeField.WEEK, LocalDateTime.class);
+ assertExtracts(DateTimeExtractField.DAY, LocalDateTimeField.DAY, LocalDateTime.class);
+ assertExtracts(DateTimeExtractField.HOUR, LocalDateTimeField.HOUR, LocalDateTime.class);
+ assertExtracts(DateTimeExtractField.MINUTE, LocalDateTimeField.MINUTE, LocalDateTime.class);
+ assertExtracts(DateTimeExtractField.SECOND, LocalDateTimeField.SECOND, LocalDateTime.class);
+ }
+
+ /**
+ * DATE and TIME are extracted as a part rather than as a field; OpenJPA supports them in
+ * JPQL only, so the criteria API must reject them with a meaningful error.
+ */
+ @Test
+ public void testUnsupportedFieldIsRejected() {
+ assertThrows(IllegalArgumentException.class,
+ () -> extract(LocalDateTimeField.DATE, LocalDateTime.class));
+ assertThrows(IllegalArgumentException.class,
+ () -> extract(LocalDateTimeField.TIME, LocalDateTime.class));
+ assertThrows(IllegalArgumentException.class,
+ () -> extract(new TemporalField<Integer, LocalDate>() { }, LocalDate.class));
+ }
+
+ private <N, T extends Temporal> void assertExtracts(DateTimeExtractField expected,
+ TemporalField<N, T> field, Class<T> temporalType) {
+ Expression<N> e = extract(field, temporalType);
+ assertSame(field.toString(), expected, ((Expressions.ExtractField<N>) e).getField());
+ assertEquals(field.toString(),
+ expected == DateTimeExtractField.SECOND ? Double.class : Integer.class, e.getJavaType());
+ }
+
+ private <N, T extends Temporal> Expression<N> extract(TemporalField<N, T> field, Class<T> temporalType) {
+ return cb.extract(field, new Expressions.Constant<>(temporalType, null));
+ }
+}