From 827554c69de52517a0b6de78a1e666fe269d82fa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gr=C3=A9gory=20Mantelet?=
 <gregory.mantelet@astro.unistra.fr>
Date: Wed, 21 Aug 2019 17:46:19 +0200
Subject: [PATCH] [ADQL] Adapt the previous commit to add OFFSET into the
 FeatureSet. With the previous commit, OFFSET was always available although it
 is actually an optional feature.

---
 src/adql/parser/ADQLQueryFactory.java        |  18 ++-
 src/adql/parser/feature/FeatureSet.java      |  13 ++-
 src/adql/parser/grammar/adqlGrammar201.jj    |  10 +-
 src/adql/query/ADQLQuery.java                |  65 ++++-------
 src/adql/query/ClauseOffset.java             | 113 +++++++++++++++++++
 src/adql/translator/JDBCTranslator.java      |   4 +-
 src/adql/translator/SQLServerTranslator.java |   8 +-
 7 files changed, 175 insertions(+), 56 deletions(-)
 create mode 100644 src/adql/query/ClauseOffset.java

diff --git a/src/adql/parser/ADQLQueryFactory.java b/src/adql/parser/ADQLQueryFactory.java
index 936dff3..4477e6f 100644
--- a/src/adql/parser/ADQLQueryFactory.java
+++ b/src/adql/parser/ADQLQueryFactory.java
@@ -28,6 +28,7 @@ import adql.parser.IdentifierItems.IdentifierItem;
 import adql.query.ADQLOrder;
 import adql.query.ADQLQuery;
 import adql.query.ClauseConstraints;
+import adql.query.ClauseOffset;
 import adql.query.ColumnReference;
 import adql.query.IdentifierField;
 import adql.query.SelectItem;
@@ -89,7 +90,7 @@ import adql.query.operand.function.string.LowerFunction;
  * </p>
  *
  * @author Gr&eacute;gory Mantelet (CDS;ARI)
- * @version 2.0 (07/2019)
+ * @version 2.0 (08/2019)
  *
  * @see ADQLParser
  */
@@ -487,4 +488,19 @@ public class ADQLQueryFactory {
 			colRef.setPosition(position);
 		return colRef;
 	}
+
+	/**
+	 * Create a {@link ClauseOffset}.
+	 *
+	 * @param offsetValue	The OFFSET's value. <b>MUST be POSITIVE</b>
+	 *
+	 * @return	The created {@link ClauseOffset}.
+	 *
+	 * @throws Exception	If the given OFFSET value is incorrect.
+	 *
+	 * @since 2.0
+	 */
+	public ClauseOffset createOffset(final int offsetValue) throws Exception {
+		return new ClauseOffset(offsetValue);
+	}
 }
diff --git a/src/adql/parser/feature/FeatureSet.java b/src/adql/parser/feature/FeatureSet.java
index 9beafbc..ba36845 100644
--- a/src/adql/parser/feature/FeatureSet.java
+++ b/src/adql/parser/feature/FeatureSet.java
@@ -28,6 +28,7 @@ import java.util.NoSuchElementException;
 import java.util.Set;
 
 import adql.db.FunctionDef;
+import adql.query.ClauseOffset;
 import adql.query.constraint.ComparisonOperator;
 import adql.query.operand.function.geometry.AreaFunction;
 import adql.query.operand.function.geometry.BoxFunction;
@@ -578,18 +579,18 @@ public class FeatureSet implements Iterable<LanguageFeature> {
 	/*public static final LanguageFeature UNION = new LanguageFeature(FeatureType.ADQL_SETS, "UNION"); // TODO UNION
 	public static final LanguageFeature EXCEPT = new LanguageFeature(FeatureType.ADQL_SETS, "EXCEPT"); // TODO EXCEPT
 	public static final LanguageFeature INTERSECT = new LanguageFeature(FeatureType.ADQL_SETS, "INTERSECT");  // TODO INTERSECT
-
+	
 	public static final LanguageFeature WITH = new LanguageFeature(FeatureType.ADQL_COMMON_TABLE, "WITH");  // TODO WITH
-
+	
 	public static final LanguageFeature CAST = new LanguageFeature(FeatureType.ADQL_TYPE, "CAST");  // TODO CAST
-
+	
 	public static final LanguageFeature IN_UNIT = new LanguageFeature(FeatureType.ADQL_UNIT, "IN_UNIT");  // TODO IN_UNIT
-
+	
 	public static final LanguageFeature BIT_AND = new LanguageFeature(FeatureType.ADQL_BITWISE, "BIT_AND");  // TODO BIT_AND
 	public static final LanguageFeature BIT_OR = new LanguageFeature(FeatureType.ADQL_BITWISE, "BIT_OR");  // TODO BIT_OR
 	public static final LanguageFeature BIT_XOR = new LanguageFeature(FeatureType.ADQL_BITWISE, "BIT_XOR");  // TODO BIT_XOR
 	public static final LanguageFeature BIT_NOT = new LanguageFeature(FeatureType.ADQL_BITWISE, "BIT_NOT");  // TODO BIT_NOT
-
+	
 	public static final LanguageFeature OFFSET = new LanguageFeature(FeatureType.ADQL_OFFSET, "OFFSET");  // TODO OFFSET*/
 
 	/** All standard features available.
@@ -602,7 +603,7 @@ public class FeatureSet implements Iterable<LanguageFeature> {
 	 * <p><i><b>Important note:</b>
 	 * 	All of them must be optional and must have a type.
 	 * </i></p> */
-	static LanguageFeature[] availableFeatures = new LanguageFeature[]{ ComparisonOperator.ILIKE.getFeatureDescription(), LowerFunction.FEATURE, AreaFunction.FEATURE, BoxFunction.FEATURE, CentroidFunction.FEATURE, CircleFunction.FEATURE, ContainsFunction.FEATURE, ExtractCoord.FEATURE_COORD1, ExtractCoord.FEATURE_COORD2, ExtractCoordSys.FEATURE, DistanceFunction.FEATURE, IntersectsFunction.FEATURE, PointFunction.FEATURE, PolygonFunction.FEATURE, RegionFunction.FEATURE };
+	static LanguageFeature[] availableFeatures = new LanguageFeature[]{ ClauseOffset.FEATURE, ComparisonOperator.ILIKE.getFeatureDescription(), LowerFunction.FEATURE, AreaFunction.FEATURE, BoxFunction.FEATURE, CentroidFunction.FEATURE, CircleFunction.FEATURE, ContainsFunction.FEATURE, ExtractCoord.FEATURE_COORD1, ExtractCoord.FEATURE_COORD2, ExtractCoordSys.FEATURE, DistanceFunction.FEATURE, IntersectsFunction.FEATURE, PointFunction.FEATURE, PolygonFunction.FEATURE, RegionFunction.FEATURE };
 
 	/**
 	 * List all available language features.
diff --git a/src/adql/parser/grammar/adqlGrammar201.jj b/src/adql/parser/grammar/adqlGrammar201.jj
index 5eb28a8..44a8a72 100644
--- a/src/adql/parser/grammar/adqlGrammar201.jj
+++ b/src/adql/parser/grammar/adqlGrammar201.jj
@@ -579,13 +579,17 @@ void OrderBy(): {ClauseADQL<ADQLOrder> orderBy = query.getOrderBy(); ADQLOrder o
 	{ orderBy.setPosition(new TextPosition(start, token)); }
 }
 
-void Offset(): { Token t; } {
-	<OFFSET> t=<UNSIGNED_INTEGER>
+void Offset(): { Token start, t; } {
+	start=<OFFSET> t=<UNSIGNED_INTEGER>
 	{
 		try{
-			query.setOffset(Integer.parseInt(t.image));
+		  	ClauseOffset offset = queryFactory.createOffset(Integer.parseInt(t.image));
+		  	offset.setPosition(new TextPosition(start, t));
+			query.setOffset(offset);
 		}catch(NumberFormatException nfe){
 			throw new ParseException("The OFFSET limit (\""+t.image+"\") isn't a regular unsigned integer!", new TextPosition(t));
+		}catch(Exception ex) {
+			throw generateParseException(ex);
 		}
 	}
 }
diff --git a/src/adql/query/ADQLQuery.java b/src/adql/query/ADQLQuery.java
index 6d1b754..45dbbc9 100644
--- a/src/adql/query/ADQLQuery.java
+++ b/src/adql/query/ADQLQuery.java
@@ -84,7 +84,7 @@ public class ADQLQuery implements ADQLObject {
 
 	/** The ADQL clause OFFSET.
 	 * @since 2.0 */
-	private int offset;
+	private ClauseOffset offset;
 
 	/** Position of this Query (or sub-query) inside the whole given ADQL query
 	 * string.
@@ -116,7 +116,7 @@ public class ADQLQuery implements ADQLObject {
 		groupBy = new ClauseADQL<ADQLColumn>("GROUP BY");
 		having = new ClauseConstraints("HAVING");
 		orderBy = new ClauseADQL<ADQLOrder>("ORDER BY");
-		offset = -1;
+		offset = null;
 	}
 
 	/**
@@ -135,7 +135,7 @@ public class ADQLQuery implements ADQLObject {
 		groupBy = (ClauseADQL<ADQLColumn>)toCopy.groupBy.getCopy();
 		having = (ClauseConstraints)toCopy.having.getCopy();
 		orderBy = (ClauseADQL<ADQLOrder>)toCopy.orderBy.getCopy();
-		offset = toCopy.offset;
+		offset = (ClauseOffset)toCopy.offset.getCopy();
 		position = (toCopy.position == null) ? null : new TextPosition(toCopy.position);
 	}
 
@@ -166,7 +166,7 @@ public class ADQLQuery implements ADQLObject {
 		groupBy.clear();
 		having.clear();
 		orderBy.clear();
-		offset = -1;
+		offset = null;
 		position = null;
 	}
 
@@ -342,40 +342,14 @@ public class ADQLQuery implements ADQLObject {
 	 * Gets the OFFSET value of this query.
 	 *
 	 * @return	Its OFFSET value,
-	 *        	or a negative value if no OFFSET is set.
+	 *        	or NULL if not OFFSET is set.
 	 *
 	 * @since 2.0
 	 */
-	public final int getOffset() {
+	public final ClauseOffset getOffset() {
 		return offset;
 	}
 
-	/**
-	 * Tell whether an OFFSET is set in this query.
-	 *
-	 * @return	<code>true</code> if an OFFSET is set,
-	 *        	<code>false</code> otherwise.
-	 *
-	 * @since 2.0
-	 */
-	public final boolean hasOffset() {
-		return (offset > -1);
-	}
-
-	/**
-	 * Remove the OFFSET value of this query.
-	 *
-	 * <p><i><b>Note:</b>
-	 * 	The position of the query is erased.
-	 * </i></p>.
-	 *
-	 * @since 2.0
-	 */
-	public void setNoOffset() {
-		offset = -1;
-		position = null;
-	}
-
 	/**
 	 * Replaces its OFFSET value by the given one.
 	 *
@@ -383,13 +357,12 @@ public class ADQLQuery implements ADQLObject {
 	 * 	The position of the query is erased.
 	 * </i></p>
 	 *
-	 * @param newOffset	The new OFFSET value.
-	 *                 	<i><b>Note:</b> a negative value removes the OFFSET from
-	 *                 	this query.</i>
+	 * @param newOffset	The new OFFSET value,
+	 *                 	or NULL to remove the current OFFSET.
 	 *
 	 * @since 2.0
 	 */
-	public void setOffset(final int newOffset) {
+	public void setOffset(final ClauseOffset newOffset) {
 		offset = newOffset;
 		position = null;
 	}
@@ -549,6 +522,9 @@ public class ADQLQuery implements ADQLObject {
 					case 5:
 						currentClause = orderBy;
 						break;
+					case 6:
+						currentClause = null;
+						return offset;
 					default:
 						throw new NoSuchElementException();
 				}
@@ -557,7 +533,7 @@ public class ADQLQuery implements ADQLObject {
 
 			@Override
 			public boolean hasNext() {
-				return index + 1 < 6;
+				return index + 1 < 7;
 			}
 
 			@Override
@@ -606,6 +582,12 @@ public class ADQLQuery implements ADQLObject {
 							else
 								throw new UnsupportedOperationException("Impossible to replace a ClauseADQL (" + orderBy.toADQL() + ") by a " + replacer.getClass().getName() + " (" + replacer.toADQL() + ")!");
 							break;
+						case 6:
+							if (replacer instanceof ClauseOffset)
+								offset = (ClauseOffset)replacer;
+							else
+								throw new UnsupportedOperationException("Impossible to replace a ClauseOffset (" + offset.toADQL() + ") by a " + replacer.getClass().getName() + " (" + replacer.toADQL() + ")!");
+							break;
 					}
 					position = null;
 				}
@@ -618,7 +600,10 @@ public class ADQLQuery implements ADQLObject {
 
 				if (index == 0 || index == 1)
 					throw new UnsupportedOperationException("Impossible to remove a " + ((index == 0) ? "SELECT" : "FROM") + " clause from a query!");
-				else {
+				else if (index == 6) {
+					offset = null;
+					position = null;
+				} else {
 					currentClause.clear();
 					position = null;
 				}
@@ -643,8 +628,8 @@ public class ADQLQuery implements ADQLObject {
 		if (!orderBy.isEmpty())
 			adql.append('\n').append(orderBy.toADQL());
 
-		if (hasOffset())
-			adql.append("\nOFFSET ").append(offset);
+		if (offset != null)
+			adql.append('\n').append(offset.toADQL());
 
 		return adql.toString();
 	}
diff --git a/src/adql/query/ClauseOffset.java b/src/adql/query/ClauseOffset.java
new file mode 100644
index 0000000..1b6bb2f
--- /dev/null
+++ b/src/adql/query/ClauseOffset.java
@@ -0,0 +1,113 @@
+package adql.query;
+
+import adql.parser.feature.LanguageFeature;
+
+/**
+ * Object representation of an OFFSET clause.
+ *
+ * <p>
+ * 	This clause is special hence the fact it does not extend
+ * 	{@link adql.query.ClauseADQL}. It contains only one value: the number of
+ * 	rows removed from the query's result head.
+ * </p>
+ *
+ * <p><i><b>Important note:</b>
+ * 	The OFFSET value stored in this object MUST always be positive.
+ * </i></p>
+ *
+ * @author Gr&eacute;gory Mantelet (CDS)
+ * @version 2.0 (08/2019)
+ * @since 2.0
+ */
+public class ClauseOffset implements ADQLObject {
+
+	/** Description of this ADQL Feature. */
+	public static final LanguageFeature FEATURE = new LanguageFeature(LanguageFeature.TYPE_ADQL_OFFSET, "OFFSET", true, "Remove the specified number of rows from the head of the query result.");
+
+	/** Name of this ADQL object. */
+	private static final String NAME = "OFFSET";
+
+	/** Value of the query's OFFSET.
+	 * <p><i><b>Important note:</b>
+	 * 	This value can never be negative.
+	 * </i></p> */
+	protected int value;
+
+	/** Position of this {@link ClauseOffset} in the original ADQL query
+	 * string. */
+	private TextPosition position = null;
+
+	/**
+	 * Create a clause OFFSET with the given offset value.
+	 *
+	 * @param offsetValue	Value of the query's result OFFSET.
+	 *
+	 * @throws IndexOutOfBoundsException	If the given value is negative.
+	 */
+	public ClauseOffset(final int offsetValue) throws IndexOutOfBoundsException {
+		setValue(offsetValue);
+	}
+
+	/**
+	 * Get the query's OFFSET.
+	 *
+	 * @return	Query's OFFSET. <i>Always positive.</i>
+	 */
+	public final int getValue() {
+		return value;
+	}
+
+	/**
+	 * Set the query's OFFSET.
+	 *
+	 * @param offsetValue	Value of the query's result OFFSET.
+	 *
+	 * @throws IndexOutOfBoundsException	If the given value is negative.
+	 */
+	public void setValue(final int offsetValue) throws IndexOutOfBoundsException {
+		if (offsetValue < 0)
+			throw new IndexOutOfBoundsException("Incorrect OFFSET value: \"" + offsetValue + "\"! It must be a positive value.");
+		this.value = offsetValue;
+	}
+
+	@Override
+	public ADQLObject getCopy() throws Exception {
+		return new ClauseOffset(value);
+	}
+
+	@Override
+	public final String getName() {
+		return NAME;
+	}
+
+	@Override
+	public final LanguageFeature getFeatureDescription() {
+		return FEATURE;
+	}
+
+	@Override
+	public final TextPosition getPosition() {
+		return position;
+	}
+
+	/**
+	 * Sets the position at which this {@link ClauseOffset} has been found in
+	 * the original ADQL query string.
+	 *
+	 * @param position	Position of this {@link ClauseOffset}.
+	 */
+	public final void setPosition(final TextPosition newPosition) {
+		position = newPosition;
+	}
+
+	@Override
+	public ADQLIterator adqlIterator() {
+		return new NullADQLIterator();
+	}
+
+	@Override
+	public String toADQL() {
+		return "OFFSET " + value;
+	}
+
+}
diff --git a/src/adql/translator/JDBCTranslator.java b/src/adql/translator/JDBCTranslator.java
index f31476b..5fe30bb 100644
--- a/src/adql/translator/JDBCTranslator.java
+++ b/src/adql/translator/JDBCTranslator.java
@@ -377,8 +377,8 @@ public abstract class JDBCTranslator implements ADQLTranslator {
 		if (query.getSelect().hasLimit())
 			sql.append("\nLIMIT ").append(query.getSelect().getLimit());
 
-		if (query.hasOffset())
-			sql.append("\nOFFSET ").append(query.getOffset());
+		if (query.getOffset() != null)
+			sql.append("\nOFFSET ").append(query.getOffset().getValue());
 
 		return sql.toString();
 	}
diff --git a/src/adql/translator/SQLServerTranslator.java b/src/adql/translator/SQLServerTranslator.java
index 21c6b9b..e52880e 100644
--- a/src/adql/translator/SQLServerTranslator.java
+++ b/src/adql/translator/SQLServerTranslator.java
@@ -161,13 +161,13 @@ public class SQLServerTranslator extends JDBCTranslator {
 			sql.append('\n').append(translate(query.getOrderBy()));
 
 		if (query.getSelect().hasLimit()) {
-			if (query.hasOffset())
-				sql.append('\n').append("OFFSET ").append(query.getOffset()).append(" ROWS");
+			if (query.getOffset() != null)
+				sql.append('\n').append("OFFSET ").append(query.getOffset().getValue()).append(" ROWS");
 			else
 				sql.append('\n').append("OFFSET 0 ROWS");
 			sql.append(" FETCH NEXT ").append(query.getSelect().getLimit()).append(" ROWS ONLY");
-		} else if (query.hasOffset()) {
-			sql.append('\n').append("OFFSET ").append(query.getOffset()).append(" ROWS");
+		} else if (query.getOffset() != null) {
+			sql.append('\n').append("OFFSET ").append(query.getOffset().getValue()).append(" ROWS");
 		}
 
 		return sql.toString();
-- 
GitLab