[GitHub] [iotdb] ericpai commented on a change in pull request #5273: [IOTDB-2679] Support logical operators in select clauses

[GitBox <gi...@apache.org>]

ericpai commented on a change in pull request #5273:
URL: https://github.com/apache/iotdb/pull/5273#discussion_r830722121



##########
File path: server/src/main/java/org/apache/iotdb/db/query/udf/core/transformer/CompareGreaterEqualTransformer.java
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.iotdb.db.query.udf.core.transformer;
+
+import org.apache.iotdb.db.query.udf.core.reader.LayerPointReader;
+
+public class CompareGreaterEqualTransformer extends CompareOperatorTransformer {
+
+  public CompareGreaterEqualTransformer(
+      LayerPointReader leftPointReader, LayerPointReader rightPointReader) {
+    super(leftPointReader, rightPointReader);
+  }
+
+  @Override
+  protected double evaluate(double leftOperand, double rightOperand) {
+    return leftOperand >= rightOperand ? 1.0 : 0.0;

Review comment:
       It's not suggested that use `>=` to compare double directly, as there may be precision loss. 
   Please reference the review comment of `CompareOperatorTransformer` and change like this
   
   ```suggestion
   protected boolean evaluate(boolean leftOperand, boolean rightOperand) {
         return Double.compare(leftOperand, rightOperand) >= 0;
   }

##########
File path: antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/SqlLexer.g4
##########
@@ -827,7 +827,8 @@ MOD : '%';
 
 // Operators. Comparation
 
-OPERATOR_EQ : '=' | '==';
+OPERATOR_DEQ : '==';

Review comment:
       What's the real difference between `==` and `=` ? Are they alias mutually?

##########
File path: server/src/main/java/org/apache/iotdb/db/query/udf/core/transformer/LogicNotTransformer.java
##########
@@ -0,0 +1,78 @@
+/*
+ * 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.iotdb.db.query.udf.core.transformer;
+
+import org.apache.iotdb.db.exception.query.QueryProcessException;
+import org.apache.iotdb.db.query.udf.core.reader.LayerPointReader;
+import org.apache.iotdb.tsfile.file.metadata.enums.TSDataType;
+
+import java.io.IOException;
+
+public class LogicNotTransformer extends Transformer {
+  private final LayerPointReader layerPointReader;
+
+  public LogicNotTransformer(LayerPointReader layerPointReader) {
+    this.layerPointReader = layerPointReader;
+  }
+
+  @Override
+  public boolean isConstantPointReader() {
+    return layerPointReader.isConstantPointReader();
+  }
+
+  @Override
+  protected boolean cacheValue() throws QueryProcessException, IOException {
+    if (!layerPointReader.next()) {
+      return false;
+    }
+    cachedTime = layerPointReader.currentTime();
+    if (layerPointReader.isCurrentNull()) {
+      currentNull = true;
+    } else {
+      switch (layerPointReader.getDataType()) {
+        case INT32:
+          cachedBoolean = layerPointReader.currentInt() == 0;
+          break;
+        case INT64:
+          cachedBoolean = layerPointReader.currentLong() == 0L;
+          break;
+        case FLOAT:
+          cachedBoolean = layerPointReader.currentFloat() == 0.0f;
+          break;
+        case DOUBLE:
+          cachedBoolean = layerPointReader.currentDouble() == 0.0;

Review comment:
       Use `Double.compare` instead.

##########
File path: server/src/main/java/org/apache/iotdb/db/query/udf/core/transformer/LogicNotTransformer.java
##########
@@ -0,0 +1,78 @@
+/*
+ * 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.iotdb.db.query.udf.core.transformer;
+
+import org.apache.iotdb.db.exception.query.QueryProcessException;
+import org.apache.iotdb.db.query.udf.core.reader.LayerPointReader;
+import org.apache.iotdb.tsfile.file.metadata.enums.TSDataType;
+
+import java.io.IOException;
+
+public class LogicNotTransformer extends Transformer {
+  private final LayerPointReader layerPointReader;
+
+  public LogicNotTransformer(LayerPointReader layerPointReader) {
+    this.layerPointReader = layerPointReader;
+  }
+
+  @Override
+  public boolean isConstantPointReader() {
+    return layerPointReader.isConstantPointReader();
+  }
+
+  @Override
+  protected boolean cacheValue() throws QueryProcessException, IOException {
+    if (!layerPointReader.next()) {
+      return false;
+    }
+    cachedTime = layerPointReader.currentTime();
+    if (layerPointReader.isCurrentNull()) {
+      currentNull = true;
+    } else {
+      switch (layerPointReader.getDataType()) {
+        case INT32:
+          cachedBoolean = layerPointReader.currentInt() == 0;
+          break;
+        case INT64:
+          cachedBoolean = layerPointReader.currentLong() == 0L;
+          break;
+        case FLOAT:
+          cachedBoolean = layerPointReader.currentFloat() == 0.0f;

Review comment:
       Use `Float.compare` instead.

##########
File path: server/src/main/java/org/apache/iotdb/db/query/udf/core/transformer/CompareEqualToTransformer.java
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.iotdb.db.query.udf.core.transformer;
+
+import org.apache.iotdb.db.query.udf.core.reader.LayerPointReader;
+
+public class CompareEqualToTransformer extends CompareOperatorTransformer {
+
+  public CompareEqualToTransformer(
+      LayerPointReader leftPointReader, LayerPointReader rightPointReader) {
+    super(leftPointReader, rightPointReader);
+  }
+
+  @Override
+  protected double evaluate(double leftOperand, double rightOperand) {
+    return leftOperand == rightOperand ? 1.0 : 0.0;

Review comment:
       It's not suggested that use `==` to compare double directly, as there may be precision loss. 
   Please reference the review comment of `CompareOperatorTransformer` and change like this
   
   ```suggestion
   protected boolean evaluate(boolean leftOperand, boolean rightOperand) {
         return Double.compare(leftOperand, rightOperand) == 0;
   }

##########
File path: server/src/main/java/org/apache/iotdb/db/query/udf/core/transformer/CompareGreaterThanTransformer.java
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.iotdb.db.query.udf.core.transformer;
+
+import org.apache.iotdb.db.query.udf.core.reader.LayerPointReader;
+
+public class CompareGreaterThanTransformer extends CompareOperatorTransformer {
+
+  public CompareGreaterThanTransformer(
+      LayerPointReader leftPointReader, LayerPointReader rightPointReader) {
+    super(leftPointReader, rightPointReader);
+  }
+
+  @Override
+  protected double evaluate(double leftOperand, double rightOperand) {
+    return leftOperand > rightOperand ? 1.0 : 0.0;

Review comment:
       Same as above.
   
   ```suggestion
   protected boolean evaluate(boolean leftOperand, boolean rightOperand) {
         return Double.compare(leftOperand, rightOperand) == 1;
   }
   ```

##########
File path: server/src/main/java/org/apache/iotdb/db/query/udf/core/transformer/CompareNonEqualTransformer.java
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.iotdb.db.query.udf.core.transformer;
+
+import org.apache.iotdb.db.query.udf.core.reader.LayerPointReader;
+
+public class CompareNonEqualTransformer extends CompareOperatorTransformer {
+
+  public CompareNonEqualTransformer(
+      LayerPointReader leftPointReader, LayerPointReader rightPointReader) {
+    super(leftPointReader, rightPointReader);
+  }
+
+  @Override
+  protected double evaluate(double leftOperand, double rightOperand) {
+    return leftOperand != rightOperand ? 1.0 : 0.0;

Review comment:
       Same as above.
   
   ```suggestion
   protected boolean evaluate(boolean leftOperand, boolean rightOperand) {
         return Double.compare(leftOperand, rightOperand) != 0;
   }
   ```

##########
File path: server/src/main/java/org/apache/iotdb/db/query/udf/core/transformer/CompareLessEqualTransformer.java
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.iotdb.db.query.udf.core.transformer;
+
+import org.apache.iotdb.db.query.udf.core.reader.LayerPointReader;
+
+public class CompareLessEqualTransformer extends CompareOperatorTransformer {
+
+  public CompareLessEqualTransformer(
+      LayerPointReader leftPointReader, LayerPointReader rightPointReader) {
+    super(leftPointReader, rightPointReader);
+  }
+
+  @Override
+  protected double evaluate(double leftOperand, double rightOperand) {
+    return leftOperand <= rightOperand ? 1.0 : 0.0;

Review comment:
       Same as above.
   
   ```suggestion
   protected boolean evaluate(boolean leftOperand, boolean rightOperand) {
         return Double.compare(leftOperand, rightOperand)  <= 0;
   }
   ```

##########
File path: server/src/main/java/org/apache/iotdb/db/query/udf/core/transformer/CompareOperatorTransformer.java
##########
@@ -0,0 +1,37 @@
+/*
+ * 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.iotdb.db.query.udf.core.transformer;
+
+import org.apache.iotdb.db.query.udf.core.reader.LayerPointReader;
+import org.apache.iotdb.tsfile.file.metadata.enums.TSDataType;
+
+public abstract class CompareOperatorTransformer extends ArithmeticBinaryTransformer {

Review comment:
       As `CompareOperatorTransformer` always accepts two inputs and output a boolean result. It's not good to inherit `ArithmeticBinaryTransformer` directly as it's return double. Maybe we can define a new base class called `LogicBinaryTransformer` which returns boolean directly?

##########
File path: server/src/main/java/org/apache/iotdb/db/query/udf/core/transformer/CompareLessThanTransformer.java
##########
@@ -0,0 +1,35 @@
+/*
+ * 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.iotdb.db.query.udf.core.transformer;
+
+import org.apache.iotdb.db.query.udf.core.reader.LayerPointReader;
+
+public class CompareLessThanTransformer extends CompareOperatorTransformer {
+
+  public CompareLessThanTransformer(
+      LayerPointReader leftPointReader, LayerPointReader rightPointReader) {
+    super(leftPointReader, rightPointReader);
+  }
+
+  @Override
+  protected double evaluate(double leftOperand, double rightOperand) {
+    return leftOperand < rightOperand ? 1.0 : 0.0;

Review comment:
       Same as above.
   
   ```suggestion
   protected boolean evaluate(boolean leftOperand, boolean rightOperand) {
         return Double.compare(leftOperand, rightOperand) < 0;
   }
   ```

##########
File path: server/src/main/java/org/apache/iotdb/db/query/udf/core/transformer/LogicAndTransformer.java
##########
@@ -0,0 +1,34 @@
+/*
+ * 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.iotdb.db.query.udf.core.transformer;
+
+import org.apache.iotdb.db.query.udf.core.reader.LayerPointReader;
+
+public class LogicAndTransformer extends CompareOperatorTransformer {
+
+  public LogicAndTransformer(LayerPointReader leftPointReader, LayerPointReader rightPointReader) {
+    super(leftPointReader, rightPointReader);
+  }
+
+  @Override
+  protected double evaluate(double leftOperand, double rightOperand) {
+    return (leftOperand == 1.0) && (rightOperand == 1.0) ? 1.0 : 0.0;

Review comment:
       Please reference the review comment of `CompareOperatorTransformer` and change like this
   
   ```suggestion
   protected boolean evaluate(boolean leftOperand, boolean rightOperand) {
         return leftOperand && rightOperand;
   }
   ```

##########
File path: server/src/main/java/org/apache/iotdb/db/query/udf/core/transformer/LogicOrTransformer.java
##########
@@ -0,0 +1,34 @@
+/*
+ * 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.iotdb.db.query.udf.core.transformer;
+
+import org.apache.iotdb.db.query.udf.core.reader.LayerPointReader;
+
+public class LogicOrTransformer extends CompareOperatorTransformer {
+
+  public LogicOrTransformer(LayerPointReader leftPointReader, LayerPointReader rightPointReader) {
+    super(leftPointReader, rightPointReader);
+  }
+
+  @Override
+  protected double evaluate(double leftOperand, double rightOperand) {

Review comment:
       Please reference the review comment of `CompareOperatorTransformer` and change like this
   
   ```suggestion
   protected boolean evaluate(boolean leftOperand, boolean rightOperand) {
         return leftOperand || rightOperand;
   }




-- 
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: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org