mqliang commented on a change in pull request #6820:
URL: https://github.com/apache/incubator-pinot/pull/6820#discussion_r644379715
##########
File path:
pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
##########
@@ -1690,6 +1700,23 @@ public void testCompilationInvokedFunction() {
greaterThan = pinotQuery.getFilterExpression().getFunctionCall();
nowTs = greaterThan.getOperands().get(1).getLiteral().getLongValue();
Assert.assertEquals(nowTs, 1577836800000L);
+
+ query = "SELECT ago('PT1H') FROM foo";
+ lowerBound = System.currentTimeMillis() - ONE_HOUR_IN_MS;
+ pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+ nowTs = pinotQuery.getSelectList().get(0).getLiteral().getLongValue();
+ upperBound = System.currentTimeMillis() - ONE_HOUR_IN_MS;
+ Assert.assertTrue(nowTs >= lowerBound);
+ Assert.assertTrue(nowTs <= upperBound);
+
+ query = "SELECT a FROM foo where time_col > ago('PT1H')";
+ lowerBound = System.currentTimeMillis() - ONE_HOUR_IN_MS;
Review comment:
Yes. L1717 verified that ago(PT1H) is invoked at compile time and the
value will get substituted
```
greaterThan = pinotQuery.getFilterExpression().getFunctionCall();
nowTs = greaterThan.getOperands().get(1).getLiteral().getLongValue();
```
##########
File path:
pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
##########
@@ -1690,6 +1700,23 @@ public void testCompilationInvokedFunction() {
greaterThan = pinotQuery.getFilterExpression().getFunctionCall();
nowTs = greaterThan.getOperands().get(1).getLiteral().getLongValue();
Assert.assertEquals(nowTs, 1577836800000L);
+
+ query = "SELECT ago('PT1H') FROM foo";
+ lowerBound = System.currentTimeMillis() - ONE_HOUR_IN_MS;
+ pinotQuery = CalciteSqlParser.compileToPinotQuery(query);
+ nowTs = pinotQuery.getSelectList().get(0).getLiteral().getLongValue();
+ upperBound = System.currentTimeMillis() - ONE_HOUR_IN_MS;
+ Assert.assertTrue(nowTs >= lowerBound);
+ Assert.assertTrue(nowTs <= upperBound);
+
+ query = "SELECT a FROM foo where time_col > ago('PT1H')";
+ lowerBound = System.currentTimeMillis() - ONE_HOUR_IN_MS;
Review comment:
> Can we also ensure that query gets rewritten ? I think the query gets
rewritten to ?
```
SELECT a FROM foo where time_col - ago('PT1H') > 0
```
I don't think the query get rewritten, If I output the complied query, it
looks like:
```
System.out.println(pinotQuery);
PinotQuery(dataSource:DataSource(tableName:foo),
selectList:[Expression(type:IDENTIFIER, identifier:Identifier(name:a))],
filterExpression:Expression(type:FUNCTION,
functionCall:Function(operator:GREATER_THAN,
operands:[Expression(type:IDENTIFIER, identifier:Identifier(name:time_col)),
Expression(type:LITERAL, literal:<Literal longValue:1622672158605>)])))
```
##########
File path:
pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
##########
@@ -373,6 +374,15 @@ public void testBrokerConverterWithLiteral() {
Assert.assertEquals(tempBrokerRequest.getQuerySource().getTableName(),
"mytable");
Assert.assertEquals(tempBrokerRequest.getSelections().getSelectionColumns().get(0),
String.format("'%s'", literal.getFieldValue().toString()));
+
+ pinotQuery = CalciteSqlParser.compileToPinotQuery("select ago('PT1H') from
mytable");
+ literal = pinotQuery.getSelectList().get(0).getLiteral();
+ Assert.assertNotNull(literal);
+ converter = new PinotQuery2BrokerRequestConverter();
+ tempBrokerRequest = converter.convert(pinotQuery);
+ Assert.assertEquals(tempBrokerRequest.getQuerySource().getTableName(),
"mytable");
+
Assert.assertEquals(tempBrokerRequest.getSelections().getSelectionColumns().get(0),
+ String.format("'%s'", literal.getFieldValue().toString()));
Review comment:
done
##########
File path:
pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/LiteralOnlyBrokerRequestTest.java
##########
@@ -76,18 +77,28 @@ public void testNumberLiteralBrokerRequestFromSQL() {
public void testLiteralOnlyTransformBrokerRequestFromSQL() {
Assert
.assertTrue(BaseBrokerRequestHandler.isLiteralOnlyQuery(CalciteSqlParser.compileToPinotQuery("SELECT
now()")));
+ Assert.assertTrue(
+
BaseBrokerRequestHandler.isLiteralOnlyQuery(CalciteSqlParser.compileToPinotQuery("SELECT
ago('PT1H')")));
Assert.assertTrue(BaseBrokerRequestHandler.isLiteralOnlyQuery(
CalciteSqlParser.compileToPinotQuery("SELECT now(),
fromDateTime('2020-01-01 UTC', 'yyyy-MM-dd z')")));
+ Assert.assertTrue(BaseBrokerRequestHandler.isLiteralOnlyQuery(
+ CalciteSqlParser.compileToPinotQuery("SELECT ago('PT1H'),
fromDateTime('2020-01-01 UTC', 'yyyy-MM-dd z')")));
Assert.assertTrue(
BaseBrokerRequestHandler.isLiteralOnlyQuery(CalciteSqlParser.compileToPinotQuery("SELECT
now() FROM myTable")));
+ Assert.assertTrue(BaseBrokerRequestHandler
+ .isLiteralOnlyQuery(CalciteSqlParser.compileToPinotQuery("SELECT
ago('PT1H') FROM myTable")));
Assert.assertTrue(BaseBrokerRequestHandler.isLiteralOnlyQuery(CalciteSqlParser
.compileToPinotQuery("SELECT now(), fromDateTime('2020-01-01 UTC',
'yyyy-MM-dd z') FROM myTable")));
+
Assert.assertTrue(BaseBrokerRequestHandler.isLiteralOnlyQuery(CalciteSqlParser
+ .compileToPinotQuery("SELECT ago('PT1H'), fromDateTime('2020-01-01
UTC', 'yyyy-MM-dd z') FROM myTable")));
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
