Merge branch 'main' into feature/json-valid

Bit-Quill · Jan 15, 2025 · dc96563 · dc96563
2 parents 2b2a8f3 + e7be8ca
commit dc96563
Show file tree

Hide file tree

Showing 12 changed files with 125 additions and 25 deletions.
diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
@@ -7,6 +7,7 @@ on:
       - '[1-9]+.[0-9x]+'
   push:
     branches-ignore:
+      - 'backport/**'
       - 'dependabot/**'
     paths:
       - '**/*.java'

diff --git a/.github/workflows/integ-tests-with-security.yml b/.github/workflows/integ-tests-with-security.yml
@@ -4,6 +4,7 @@ on:
   pull_request:
   push:
     branches-ignore:
+      - 'backport/**'
       - 'dependabot/**'
     paths:
       - 'integ-test/**'

diff --git a/.github/workflows/sql-test-and-build-workflow.yml b/.github/workflows/sql-test-and-build-workflow.yml
@@ -4,6 +4,7 @@ on:
   pull_request:
   push:
     branches-ignore:
+      - 'backport/**'
       - 'dependabot/**'
     paths:
       - '**/*.java'
@@ -51,7 +52,7 @@ jobs:
     - name: Build with Gradle
       run: |
         chown -R 1000:1000 `pwd`
-        su `id -un 1000` -c "./gradlew --continue --parallel build"
+        su `id -un 1000` -c "./gradlew --continue build"
 
     - name: Create Artifact Path
       run: |
@@ -113,7 +114,7 @@ jobs:
         java-version: ${{ matrix.entry.java }}
 
     - name: Build with Gradle
-      run: ./gradlew --continue --parallel build ${{ matrix.entry.os_build_args }}
+      run: ./gradlew --continue build ${{ matrix.entry.os_build_args }}
 
     - name: Create Artifact Path
       run: |

diff --git a/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java b/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java
@@ -356,8 +356,7 @@ public Expression visitWhen(When node, AnalysisContext context) {
 
   @Override
   public Expression visitField(Field node, AnalysisContext context) {
-    String attr = node.getField().toString();
-    return visitIdentifier(attr, context);
+    return visitQualifiedName((QualifiedName) node.getField(), context);
   }
 
   @Override

diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java
@@ -5,7 +5,8 @@
 
 package org.opensearch.sql.expression.datetime;
 
-import static java.time.temporal.ChronoField.ALIGNED_WEEK_OF_YEAR;
+import static java.time.DayOfWeek.SUNDAY;
+import static java.time.temporal.TemporalAdjusters.nextOrSame;
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -24,6 +25,7 @@
 import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.format.DateTimeFormatter;
+import java.time.temporal.ChronoUnit;
 import java.util.List;
 import java.util.stream.Stream;
 import lombok.AllArgsConstructor;
@@ -1228,30 +1230,34 @@ public void testWeekFormats(
         expectedInteger);
   }
 
-  // subtracting 1 as a temporary fix for year 2024.
-  // Issue: https://github.com/opensearch-project/sql/issues/2477
   @Test
   public void testWeekOfYearWithTimeType() {
+    LocalDate today = LocalDate.now(functionProperties.getQueryStartClock());
+
+    // week is based on the first sunday of the year
+    LocalDate firstSundayOfYear = today.withDayOfYear(1).with(nextOrSame(SUNDAY));
+    int week =
+        today.isBefore(firstSundayOfYear)
+            ? 0
+            : (int) ChronoUnit.WEEKS.between(firstSundayOfYear, today) + 1;
+
     assertAll(
         () ->
             validateStringFormat(
                 DSL.week(
                     functionProperties, DSL.literal(new ExprTimeValue("12:23:34")), DSL.literal(0)),
                 "week(TIME '12:23:34', 0)",
-                LocalDate.now(functionProperties.getQueryStartClock()).get(ALIGNED_WEEK_OF_YEAR)
-                    - 1),
+                week),
         () ->
             validateStringFormat(
                 DSL.week_of_year(functionProperties, DSL.literal(new ExprTimeValue("12:23:34"))),
                 "week_of_year(TIME '12:23:34')",
-                LocalDate.now(functionProperties.getQueryStartClock()).get(ALIGNED_WEEK_OF_YEAR)
-                    - 1),
+                week),
         () ->
             validateStringFormat(
                 DSL.weekofyear(functionProperties, DSL.literal(new ExprTimeValue("12:23:34"))),
                 "weekofyear(TIME '12:23:34')",
-                LocalDate.now(functionProperties.getQueryStartClock()).get(ALIGNED_WEEK_OF_YEAR)
-                    - 1));
+                week));
   }
 
   @Test

diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java
@@ -5,14 +5,16 @@
 
 package org.opensearch.sql.expression.datetime;
 
-import static java.time.temporal.ChronoField.ALIGNED_WEEK_OF_YEAR;
+import static java.time.DayOfWeek.SUNDAY;
+import static java.time.temporal.TemporalAdjusters.nextOrSame;
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.opensearch.sql.data.model.ExprValueUtils.integerValue;
 import static org.opensearch.sql.data.type.ExprCoreType.INTEGER;
 
 import java.time.LocalDate;
+import java.time.temporal.ChronoUnit;
 import java.util.stream.Stream;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -97,13 +99,9 @@ public void testYearweekWithoutMode() {
     assertEquals(eval(expression), eval(expressionWithoutMode));
   }
 
-  // subtracting 1 as a temporary fix for year 2024.
-  // Issue: https://github.com/opensearch-project/sql/issues/2477
   @Test
   public void testYearweekWithTimeType() {
-    int week = LocalDate.now(functionProperties.getQueryStartClock()).get(ALIGNED_WEEK_OF_YEAR) - 1;
-    int year = LocalDate.now(functionProperties.getQueryStartClock()).getYear();
-    int expected = Integer.parseInt(String.format("%d%02d", year, week));
+    int expected = getYearWeekBeforeSunday(LocalDate.now(functionProperties.getQueryStartClock()));
 
     FunctionExpression expression =
         DSL.yearweek(
@@ -112,9 +110,27 @@ public void testYearweekWithTimeType() {
     FunctionExpression expressionWithoutMode =
         DSL.yearweek(functionProperties, DSL.literal(new ExprTimeValue("10:11:12")));
 
-    assertAll(
-        () -> assertEquals(expected, eval(expression).integerValue()),
-        () -> assertEquals(expected, eval(expressionWithoutMode).integerValue()));
+    assertEquals(
+        expected,
+        eval(expression).integerValue(),
+        String.format(
+            "Expected year week: %d, got %s (test with mode)", expected, eval(expression)));
+    assertEquals(
+        expected,
+        eval(expressionWithoutMode).integerValue(),
+        String.format(
+            "Expected year week: %d, got %s (test without mode)", expected, eval(expression)));
+  }
+
+  private int getYearWeekBeforeSunday(LocalDate date) {
+    LocalDate firstSundayOfYear = date.withDayOfYear(1).with(nextOrSame(SUNDAY));
+    if (date.isBefore(firstSundayOfYear)) {
+      return getYearWeekBeforeSunday(date.minusDays(1));
+    }
+
+    int week = (int) ChronoUnit.WEEKS.between(firstSundayOfYear, date) + 1;
+    int year = date.getYear();
+    return Integer.parseInt(String.format("%d%02d", year, week));
   }
 
   @Test

diff --git a/docs/user/ppl/functions/datetime.rst b/docs/user/ppl/functions/datetime.rst
@@ -2169,7 +2169,7 @@ YEARWEEK
 Description
 >>>>>>>>>>>
 
-Usage: yearweek(date) returns the year and week for date as an integer. It accepts and optional mode arguments aligned with those available for the `WEEK`_ function.
+Usage: yearweek(date[, mode]) returns the year and week for date as an integer. It accepts and optional mode arguments aligned with those available for the `WEEK`_ function.
 
 Argument type: STRING/DATE/TIME/TIMESTAMP
 
@@ -2185,4 +2185,3 @@ Example::
     | 202034                 | 201901                    |
     +------------------------+---------------------------+
 
-
diff --git a/docs/user/ppl/general/identifiers.rst b/docs/user/ppl/general/identifiers.rst
@@ -161,3 +161,28 @@ Query delimited multiple indices seperated by ``,``::
     | 5       |
     +---------+
 
+Metadata Identifiers
+====================
+
+Description
+-----------
+
+One can also provide meta-field name(s) to retrieve reserved-fields (beginning with underscore) from OpenSearch documents. Meta-fields are not output
+as default field list (`search source=<index>`) and must be explicitly included to be returned.
+
+Examples
+---------
+
+Query metadata fields::
+
+    os> source=accounts | fields firstname, lastname, _index, _sort;
+    fetched rows / total rows = 4/4
+    +-----------+----------+----------+-------+
+    | firstname | lastname | _index   | _sort |
+    |-----------+----------+----------+-------|
+    | Amber     | Duke     | accounts | -2    |
+    | Hattie    | Bond     | accounts | -2    |
+    | Nanette   | Bates    | accounts | -2    |
+    | Dale      | Adams    | accounts | -2    |
+    +-----------+----------+----------+-------+
+
diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/FieldsCommandIT.java
@@ -68,4 +68,19 @@ public void testSelectDateTypeField() throws IOException {
         rows("2018-08-19 00:00:00"),
         rows("2018-08-11 00:00:00"));
   }
+
+  @Test
+  public void testMetadataFields() throws IOException {
+    JSONObject result =
+        executeQuery(String.format("source=%s | fields firstname, _index", TEST_INDEX_ACCOUNT));
+    verifyColumn(result, columnName("firstname"), columnName("_index"));
+  }
+
+  @Test
+  public void testDelimitedMetadataFields() throws IOException {
+    JSONObject result =
+        executeQuery(
+            String.format("source=%s | fields firstname, `_id`, `_index`", TEST_INDEX_ACCOUNT));
+    verifyColumn(result, columnName("firstname"), columnName("_id"), columnName("_index"));
+  }
 }
diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java
@@ -98,4 +98,12 @@ public void testIsNotNullFunction() throws IOException {
                 TEST_INDEX_BANK_WITH_NULL_VALUES));
     verifyDataRows(result, rows("Amber JOHnny"));
   }
+
+  @Test
+  public void testWhereWithMetadataFields() throws IOException {
+    JSONObject result =
+        executeQuery(
+            String.format("source=%s | where _id='1' | fields firstname", TEST_INDEX_ACCOUNT));
+    verifyDataRows(result, rows("Amber"));
+  }
 }
diff --git a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 b/ppl/src/main/antlr/OpenSearchPPLLexer.g4
@@ -401,13 +401,16 @@ INTEGER_LITERAL:                    DEC_DIGIT+;
 DECIMAL_LITERAL:                    (DEC_DIGIT+)? '.' DEC_DIGIT+;
 
 fragment DATE_SUFFIX:               ([\-.][*0-9]+)+;
-fragment ID_LITERAL:                [@*A-Z]+?[*A-Z_\-0-9]*;
 fragment CLUSTER_PREFIX_LITERAL:    [*A-Z]+?[*A-Z_\-0-9]* COLON;
 ID_DATE_SUFFIX:                     CLUSTER_PREFIX_LITERAL? ID_LITERAL DATE_SUFFIX;
 DQUOTA_STRING:                      '"' ( '\\'. | '""' | ~('"'| '\\') )* '"';
 SQUOTA_STRING:                      '\'' ('\\'. | '\'\'' | ~('\'' | '\\'))* '\'';
 BQUOTA_STRING:                      '`' ( '\\'. | '``' | ~('`'|'\\'))* '`';
 fragment DEC_DIGIT:                 [0-9];
 
+// Identifiers cannot start with a single '_' since this an OpenSearch reserved
+// metadata field.  Two underscores (or more) is acceptable, such as '__field'.
+fragment ID_LITERAL:                ([@*A-Z_])+?[*A-Z_\-0-9]*;
+
 
 ERROR_RECOGNITION:                  .    -> channel(ERRORCHANNEL);
diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java
@@ -649,6 +649,32 @@ public void canBuildKeywordsAsIdentInQualifiedName() {
         projectWithArg(relation("test"), defaultFieldsArgs(), field("timestamp")));
   }
 
+  @Test
+  public void canBuildMetaDataFieldAsQualifiedName() {
+    assertEqual(
+        "source=test | fields _id, _index, _sort, _maxscore",
+        projectWithArg(
+            relation("test"),
+            defaultFieldsArgs(),
+            field("_id"),
+            field("_index"),
+            field("_sort"),
+            field("_maxscore")));
+  }
+
+  @Test
+  public void canBuildNonMetaDataFieldAsQualifiedName() {
+    assertEqual(
+        "source=test | fields id, __id, _routing, ___field",
+        projectWithArg(
+            relation("test"),
+            defaultFieldsArgs(),
+            field("id"),
+            field("__id"),
+            field("_routing"),
+            field("___field")));
+  }
+
   @Test
   public void canBuildMatchRelevanceFunctionWithArguments() {
     assertEqual(