Details

      Activity

      Hide
      hudson Hudson added a comment -

      SUCCESS: Integrated in Tajo-master-build #638 (See https://builds.apache.org/job/Tajo-master-build/638/)
      TAJO-921: Add STDDEV_SAMP and STDDEV_POP window functions. (Keuntae Park) (sirpkt: rev 652e4db7965e6452ad2a9778f8383141bb63e22f)

      • tajo-core/src/test/resources/queries/TestWindowQuery/testFirstValue1.sql
      • tajo-core/src/test/resources/results/TestWindowQuery/testRowNumber3.result
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDev.java
      • tajo-core/src/test/java/org/apache/tajo/engine/query/TestWindowQuery.java
      • tajo-core/src/test/resources/results/TestWindowQuery/testRowNumber1.result
      • tajo-core/src/test/resources/results/TestWindowQuery/testLastValue1.result
      • tajo-core/src/test/resources/results/TestWindowQuery/lastValue1.result
      • tajo-core/src/test/resources/queries/TestWindowQuery/testRowNumber3.sql
      • tajo-core/src/test/resources/results/TestWindowQuery/rowNumber3.result
      • tajo-core/src/test/resources/results/TestWindowQuery/testStdDevPop1.result
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampInt.java
      • tajo-core/src/test/resources/queries/TestWindowQuery/testStdDevSamp1.sql
      • tajo-core/src/test/resources/results/TestWindowQuery/testStdDevSamp1.result
      • tajo-core/src/test/resources/queries/TestWindowQuery/firstValue1.sql
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopFloat.java
      • tajo-core/src/test/resources/results/TestWindowQuery/rowNumber2.result
      • tajo-core/src/main/proto/InternalTypes.proto
      • tajo-core/src/test/resources/queries/TestWindowQuery/lastValue1.sql
      • tajo-core/src/test/resources/queries/TestWindowQuery/testStdDevPop1.sql
      • tajo-core/src/test/resources/queries/TestWindowQuery/rowNumber1.sql
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampLong.java
      • tajo-core/src/test/resources/results/TestWindowQuery/rowNumber1.result
      • CHANGES
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampFloat.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopInt.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopLong.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopDouble.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampDouble.java
      • tajo-core/src/test/java/org/apache/tajo/engine/function/TestBuiltinFunctions.java
      • tajo-core/src/test/resources/queries/TestWindowQuery/rowNumber3.sql
      • tajo-core/src/test/resources/results/TestWindowQuery/testFirstValue1.result
      • tajo-core/src/test/resources/results/TestWindowQuery/testRowNumber2.result
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSamp.java
      • tajo-core/src/test/resources/results/TestWindowQuery/firstValue1.result
      • tajo-core/src/test/resources/queries/TestWindowQuery/testRowNumber2.sql
      • tajo-core/src/test/resources/queries/TestWindowQuery/rowNumber2.sql
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPop.java
      • tajo-core/src/test/resources/queries/TestWindowQuery/testRowNumber1.sql
      • tajo-core/src/test/resources/queries/TestWindowQuery/testLastValue1.sql
      Show
      hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #638 (See https://builds.apache.org/job/Tajo-master-build/638/ ) TAJO-921 : Add STDDEV_SAMP and STDDEV_POP window functions. (Keuntae Park) (sirpkt: rev 652e4db7965e6452ad2a9778f8383141bb63e22f) tajo-core/src/test/resources/queries/TestWindowQuery/testFirstValue1.sql tajo-core/src/test/resources/results/TestWindowQuery/testRowNumber3.result tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDev.java tajo-core/src/test/java/org/apache/tajo/engine/query/TestWindowQuery.java tajo-core/src/test/resources/results/TestWindowQuery/testRowNumber1.result tajo-core/src/test/resources/results/TestWindowQuery/testLastValue1.result tajo-core/src/test/resources/results/TestWindowQuery/lastValue1.result tajo-core/src/test/resources/queries/TestWindowQuery/testRowNumber3.sql tajo-core/src/test/resources/results/TestWindowQuery/rowNumber3.result tajo-core/src/test/resources/results/TestWindowQuery/testStdDevPop1.result tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampInt.java tajo-core/src/test/resources/queries/TestWindowQuery/testStdDevSamp1.sql tajo-core/src/test/resources/results/TestWindowQuery/testStdDevSamp1.result tajo-core/src/test/resources/queries/TestWindowQuery/firstValue1.sql tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopFloat.java tajo-core/src/test/resources/results/TestWindowQuery/rowNumber2.result tajo-core/src/main/proto/InternalTypes.proto tajo-core/src/test/resources/queries/TestWindowQuery/lastValue1.sql tajo-core/src/test/resources/queries/TestWindowQuery/testStdDevPop1.sql tajo-core/src/test/resources/queries/TestWindowQuery/rowNumber1.sql tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampLong.java tajo-core/src/test/resources/results/TestWindowQuery/rowNumber1.result CHANGES tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampFloat.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopInt.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopLong.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopDouble.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampDouble.java tajo-core/src/test/java/org/apache/tajo/engine/function/TestBuiltinFunctions.java tajo-core/src/test/resources/queries/TestWindowQuery/rowNumber3.sql tajo-core/src/test/resources/results/TestWindowQuery/testFirstValue1.result tajo-core/src/test/resources/results/TestWindowQuery/testRowNumber2.result tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSamp.java tajo-core/src/test/resources/results/TestWindowQuery/firstValue1.result tajo-core/src/test/resources/queries/TestWindowQuery/testRowNumber2.sql tajo-core/src/test/resources/queries/TestWindowQuery/rowNumber2.sql tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPop.java tajo-core/src/test/resources/queries/TestWindowQuery/testRowNumber1.sql tajo-core/src/test/resources/queries/TestWindowQuery/testLastValue1.sql
      Hide
      hudson Hudson added a comment -

      ABORTED: Integrated in Tajo-master-CODEGEN-build #275 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/275/)
      TAJO-921: Add STDDEV_SAMP and STDDEV_POP window functions. (Keuntae Park) (sirpkt: rev 652e4db7965e6452ad2a9778f8383141bb63e22f)

      • tajo-core/src/main/proto/InternalTypes.proto
      • tajo-core/src/test/resources/queries/TestWindowQuery/firstValue1.sql
      • tajo-core/src/test/resources/results/TestWindowQuery/rowNumber1.result
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopInt.java
      • tajo-core/src/test/resources/results/TestWindowQuery/testRowNumber2.result
      • tajo-core/src/test/resources/results/TestWindowQuery/firstValue1.result
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopFloat.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPop.java
      • tajo-core/src/test/java/org/apache/tajo/engine/query/TestWindowQuery.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampDouble.java
      • CHANGES
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampFloat.java
      • tajo-core/src/test/resources/results/TestWindowQuery/lastValue1.result
      • tajo-core/src/test/resources/results/TestWindowQuery/testRowNumber3.result
      • tajo-core/src/test/resources/queries/TestWindowQuery/testRowNumber3.sql
      • tajo-core/src/test/resources/queries/TestWindowQuery/testStdDevPop1.sql
      • tajo-core/src/test/resources/results/TestWindowQuery/testStdDevPop1.result
      • tajo-core/src/test/resources/results/TestWindowQuery/testRowNumber1.result
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSamp.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampLong.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampInt.java
      • tajo-core/src/test/resources/queries/TestWindowQuery/testRowNumber1.sql
      • tajo-core/src/test/java/org/apache/tajo/engine/function/TestBuiltinFunctions.java
      • tajo-core/src/test/resources/queries/TestWindowQuery/testFirstValue1.sql
      • tajo-core/src/test/resources/results/TestWindowQuery/testFirstValue1.result
      • tajo-core/src/test/resources/queries/TestWindowQuery/rowNumber3.sql
      • tajo-core/src/test/resources/queries/TestWindowQuery/lastValue1.sql
      • tajo-core/src/test/resources/queries/TestWindowQuery/rowNumber1.sql
      • tajo-core/src/test/resources/results/TestWindowQuery/testLastValue1.result
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopDouble.java
      • tajo-core/src/test/resources/queries/TestWindowQuery/rowNumber2.sql
      • tajo-core/src/test/resources/results/TestWindowQuery/testStdDevSamp1.result
      • tajo-core/src/test/resources/queries/TestWindowQuery/testStdDevSamp1.sql
      • tajo-core/src/test/resources/queries/TestWindowQuery/testRowNumber2.sql
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopLong.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDev.java
      • tajo-core/src/test/resources/queries/TestWindowQuery/testLastValue1.sql
      • tajo-core/src/test/resources/results/TestWindowQuery/rowNumber3.result
      • tajo-core/src/test/resources/results/TestWindowQuery/rowNumber2.result
      Show
      hudson Hudson added a comment - ABORTED: Integrated in Tajo-master-CODEGEN-build #275 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/275/ ) TAJO-921 : Add STDDEV_SAMP and STDDEV_POP window functions. (Keuntae Park) (sirpkt: rev 652e4db7965e6452ad2a9778f8383141bb63e22f) tajo-core/src/main/proto/InternalTypes.proto tajo-core/src/test/resources/queries/TestWindowQuery/firstValue1.sql tajo-core/src/test/resources/results/TestWindowQuery/rowNumber1.result tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopInt.java tajo-core/src/test/resources/results/TestWindowQuery/testRowNumber2.result tajo-core/src/test/resources/results/TestWindowQuery/firstValue1.result tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopFloat.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPop.java tajo-core/src/test/java/org/apache/tajo/engine/query/TestWindowQuery.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampDouble.java CHANGES tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampFloat.java tajo-core/src/test/resources/results/TestWindowQuery/lastValue1.result tajo-core/src/test/resources/results/TestWindowQuery/testRowNumber3.result tajo-core/src/test/resources/queries/TestWindowQuery/testRowNumber3.sql tajo-core/src/test/resources/queries/TestWindowQuery/testStdDevPop1.sql tajo-core/src/test/resources/results/TestWindowQuery/testStdDevPop1.result tajo-core/src/test/resources/results/TestWindowQuery/testRowNumber1.result tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSamp.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampLong.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSampInt.java tajo-core/src/test/resources/queries/TestWindowQuery/testRowNumber1.sql tajo-core/src/test/java/org/apache/tajo/engine/function/TestBuiltinFunctions.java tajo-core/src/test/resources/queries/TestWindowQuery/testFirstValue1.sql tajo-core/src/test/resources/results/TestWindowQuery/testFirstValue1.result tajo-core/src/test/resources/queries/TestWindowQuery/rowNumber3.sql tajo-core/src/test/resources/queries/TestWindowQuery/lastValue1.sql tajo-core/src/test/resources/queries/TestWindowQuery/rowNumber1.sql tajo-core/src/test/resources/results/TestWindowQuery/testLastValue1.result tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopDouble.java tajo-core/src/test/resources/queries/TestWindowQuery/rowNumber2.sql tajo-core/src/test/resources/results/TestWindowQuery/testStdDevSamp1.result tajo-core/src/test/resources/queries/TestWindowQuery/testStdDevSamp1.sql tajo-core/src/test/resources/queries/TestWindowQuery/testRowNumber2.sql tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPopLong.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDev.java tajo-core/src/test/resources/queries/TestWindowQuery/testLastValue1.sql tajo-core/src/test/resources/results/TestWindowQuery/rowNumber3.result tajo-core/src/test/resources/results/TestWindowQuery/rowNumber2.result
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user sirpkt commented on the pull request:

      https://github.com/apache/tajo/pull/427#issuecomment-87563600

      Thank you for the review, @jihoonson

      I just committed the patch to master.

      Show
      githubbot ASF GitHub Bot added a comment - Github user sirpkt commented on the pull request: https://github.com/apache/tajo/pull/427#issuecomment-87563600 Thank you for the review, @jihoonson I just committed the patch to master.
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user asfgit closed the pull request at:

      https://github.com/apache/tajo/pull/427

      Show
      githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/427
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user jihoonson commented on the pull request:

      https://github.com/apache/tajo/pull/427#issuecomment-87560832

      +1
      Thanks for your nice work!

      Show
      githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/427#issuecomment-87560832 +1 Thanks for your nice work!
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user jihoonson commented on the pull request:

      https://github.com/apache/tajo/pull/427#issuecomment-87549317

      @sirpkt thanks for quick response.
      I'll finish my review soon.

      Show
      githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/427#issuecomment-87549317 @sirpkt thanks for quick response. I'll finish my review soon.
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user sirpkt commented on the pull request:

      https://github.com/apache/tajo/pull/427#issuecomment-87546262

      Hi, @jihoonson
      Above difference is from the handling of default window frame value.
      In Tajo, current window functions works as if window frame is set as "rows between unbounded preceding and unbounded following"
      while default setting of all other DBMSs is "range between unbounded preceding and current row"

      I already posted patch to support correct window frame as TAJO-1415.
      So, after that patch, the result will be the same.
      Or you can test with window frame setting of "rows between unbounded preceding and unbounded following" for all window functions.

      Show
      githubbot ASF GitHub Bot added a comment - Github user sirpkt commented on the pull request: https://github.com/apache/tajo/pull/427#issuecomment-87546262 Hi, @jihoonson Above difference is from the handling of default window frame value. In Tajo, current window functions works as if window frame is set as "rows between unbounded preceding and unbounded following" while default setting of all other DBMSs is "range between unbounded preceding and current row" I already posted patch to support correct window frame as TAJO-1415 . So, after that patch, the result will be the same. Or you can test with window frame setting of "rows between unbounded preceding and unbounded following" for all window functions.
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user jihoonson commented on a diff in the pull request:

      https://github.com/apache/tajo/pull/427#discussion_r27366512

      — Diff: tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDev.java —
      @@ -0,0 +1,94 @@
      +/**
      + * 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.tajo.engine.function.builtin;
      +
      +import org.apache.tajo.catalog.CatalogUtil;
      +import org.apache.tajo.catalog.Column;
      +import org.apache.tajo.common.TajoDataTypes.DataType;
      +import org.apache.tajo.common.TajoDataTypes.Type;
      +import org.apache.tajo.datum.Datum;
      +import org.apache.tajo.datum.NullDatum;
      +import org.apache.tajo.datum.ProtobufDatum;
      +import org.apache.tajo.plan.function.AggFunction;
      +import org.apache.tajo.plan.function.FunctionContext;
      +import org.apache.tajo.storage.Tuple;
      +
      +import static org.apache.tajo.InternalTypes.StdDevProto;
      +
      +public abstract class StdDev extends AggFunction<Datum> {
      +
      + public StdDev(Column[] definedArgs)

      { + super(definedArgs); + }

      +
      + public StdDevContext newContext()

      { + return new StdDevContext(); + }

      +
      + @Override
      + public void eval(FunctionContext ctx, Tuple params) {
      + StdDevContext StdDevCtx = (StdDevContext) ctx;
      + Datum datum = params.get(0);
      + if (datum.isNotNull()) {
      + double delta = datum.asFloat8() - StdDevCtx.avg;
      + StdDevCtx.count++;
      + StdDevCtx.avg += delta/StdDevCtx.count;
      + StdDevCtx.squareSumOfDiff += delta * (datum.asFloat8() - StdDevCtx.avg);
      — End diff –

      You are right. Sorry for my mistake.

      Show
      githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/427#discussion_r27366512 — Diff: tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDev.java — @@ -0,0 +1,94 @@ +/** + * 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.tajo.engine.function.builtin; + +import org.apache.tajo.catalog.CatalogUtil; +import org.apache.tajo.catalog.Column; +import org.apache.tajo.common.TajoDataTypes.DataType; +import org.apache.tajo.common.TajoDataTypes.Type; +import org.apache.tajo.datum.Datum; +import org.apache.tajo.datum.NullDatum; +import org.apache.tajo.datum.ProtobufDatum; +import org.apache.tajo.plan.function.AggFunction; +import org.apache.tajo.plan.function.FunctionContext; +import org.apache.tajo.storage.Tuple; + +import static org.apache.tajo.InternalTypes.StdDevProto; + +public abstract class StdDev extends AggFunction<Datum> { + + public StdDev(Column[] definedArgs) { + super(definedArgs); + } + + public StdDevContext newContext() { + return new StdDevContext(); + } + + @Override + public void eval(FunctionContext ctx, Tuple params) { + StdDevContext StdDevCtx = (StdDevContext) ctx; + Datum datum = params.get(0); + if (datum.isNotNull()) { + double delta = datum.asFloat8() - StdDevCtx.avg; + StdDevCtx.count++; + StdDevCtx.avg += delta/StdDevCtx.count; + StdDevCtx.squareSumOfDiff += delta * (datum.asFloat8() - StdDevCtx.avg); — End diff – You are right. Sorry for my mistake.
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user sirpkt commented on the pull request:

      https://github.com/apache/tajo/pull/427#issuecomment-87542931

      Thank you for the comment, @jihoonson
      I'll check about it.

      Show
      githubbot ASF GitHub Bot added a comment - Github user sirpkt commented on the pull request: https://github.com/apache/tajo/pull/427#issuecomment-87542931 Thank you for the comment, @jihoonson I'll check about it.
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user jihoonson commented on the pull request:

      https://github.com/apache/tajo/pull/427#issuecomment-87540688

      Hi @sirpkt, thanks for your patch.
      I executed the same queries in newly added unit tests with pgsql, but got different results.
      Would you check it? Here are the results of pgsql.

      • result of testStdDevPop1
        ```
        linenumber_stddev_pop | suppkey_stddev_pop | extendedprice_stddev_pop | discount_stddev_pop
        -----------------------------------------------------------------+--------------------
        0 | 0 | 0 | 0
        0.50000000000000000000 | 197.500000000000 | 12407.465 | 0.0250000022351742
        0 | 0 | 0 | 0
        0 | 0 | 0 | 0
        0.50000000000000000000 | 2371.000000000000 | 3630.79000000004 | 0.0200000014156103
        (5 rows)
        ```
      • result of testStdDevSamp1
        ```
        linenumber_stddev_samp | suppkey_stddev_samp | extendedprice_stddev_samp | discount_stddev_samp
        -------------------------------------------------------------------+---------------------
           

        0.70710678118654752440 | 279.307178568686 | 17546.8052776695 | 0.035355342220341

           
           

        0.70710678118654752440 | 3353.100356386608 | 5134.71246012867 | 0.0282842732494372
        (5 rows)
        ```

      Show
      githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/427#issuecomment-87540688 Hi @sirpkt, thanks for your patch. I executed the same queries in newly added unit tests with pgsql, but got different results. Would you check it? Here are the results of pgsql. result of testStdDevPop1 ``` linenumber_stddev_pop | suppkey_stddev_pop | extendedprice_stddev_pop | discount_stddev_pop ----------------------- ------------------ ------------------------ + -------------------- 0 | 0 | 0 | 0 0.50000000000000000000 | 197.500000000000 | 12407.465 | 0.0250000022351742 0 | 0 | 0 | 0 0 | 0 | 0 | 0 0.50000000000000000000 | 2371.000000000000 | 3630.79000000004 | 0.0200000014156103 (5 rows) ``` result of testStdDevSamp1 ``` linenumber_stddev_samp | suppkey_stddev_samp | extendedprice_stddev_samp | discount_stddev_samp ----------------------- ------------------- ------------------------- + ---------------------     0.70710678118654752440 | 279.307178568686 | 17546.8052776695 | 0.035355342220341         0.70710678118654752440 | 3353.100356386608 | 5134.71246012867 | 0.0282842732494372 (5 rows) ```
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user sirpkt commented on a diff in the pull request:

      https://github.com/apache/tajo/pull/427#discussion_r27366399

      — Diff: tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDev.java —
      @@ -0,0 +1,94 @@
      +/**
      + * 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.tajo.engine.function.builtin;
      +
      +import org.apache.tajo.catalog.CatalogUtil;
      +import org.apache.tajo.catalog.Column;
      +import org.apache.tajo.common.TajoDataTypes.DataType;
      +import org.apache.tajo.common.TajoDataTypes.Type;
      +import org.apache.tajo.datum.Datum;
      +import org.apache.tajo.datum.NullDatum;
      +import org.apache.tajo.datum.ProtobufDatum;
      +import org.apache.tajo.plan.function.AggFunction;
      +import org.apache.tajo.plan.function.FunctionContext;
      +import org.apache.tajo.storage.Tuple;
      +
      +import static org.apache.tajo.InternalTypes.StdDevProto;
      +
      +public abstract class StdDev extends AggFunction<Datum> {
      +
      + public StdDev(Column[] definedArgs)

      { + super(definedArgs); + }

      +
      + public StdDevContext newContext()

      { + return new StdDevContext(); + }

      +
      + @Override
      + public void eval(FunctionContext ctx, Tuple params) {
      + StdDevContext StdDevCtx = (StdDevContext) ctx;
      + Datum datum = params.get(0);
      + if (datum.isNotNull()) {
      + double delta = datum.asFloat8() - StdDevCtx.avg;
      + StdDevCtx.count++;
      + StdDevCtx.avg += delta/StdDevCtx.count;
      + StdDevCtx.squareSumOfDiff += delta * (datum.asFloat8() - StdDevCtx.avg);
      — End diff –

      Actually, datum.asFloat8() - StdDevCtx.avg in line 52 is not equal to delta
      because StdDevCtx.avg is updated after delta is calculated.

      Show
      githubbot ASF GitHub Bot added a comment - Github user sirpkt commented on a diff in the pull request: https://github.com/apache/tajo/pull/427#discussion_r27366399 — Diff: tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDev.java — @@ -0,0 +1,94 @@ +/** + * 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.tajo.engine.function.builtin; + +import org.apache.tajo.catalog.CatalogUtil; +import org.apache.tajo.catalog.Column; +import org.apache.tajo.common.TajoDataTypes.DataType; +import org.apache.tajo.common.TajoDataTypes.Type; +import org.apache.tajo.datum.Datum; +import org.apache.tajo.datum.NullDatum; +import org.apache.tajo.datum.ProtobufDatum; +import org.apache.tajo.plan.function.AggFunction; +import org.apache.tajo.plan.function.FunctionContext; +import org.apache.tajo.storage.Tuple; + +import static org.apache.tajo.InternalTypes.StdDevProto; + +public abstract class StdDev extends AggFunction<Datum> { + + public StdDev(Column[] definedArgs) { + super(definedArgs); + } + + public StdDevContext newContext() { + return new StdDevContext(); + } + + @Override + public void eval(FunctionContext ctx, Tuple params) { + StdDevContext StdDevCtx = (StdDevContext) ctx; + Datum datum = params.get(0); + if (datum.isNotNull()) { + double delta = datum.asFloat8() - StdDevCtx.avg; + StdDevCtx.count++; + StdDevCtx.avg += delta/StdDevCtx.count; + StdDevCtx.squareSumOfDiff += delta * (datum.asFloat8() - StdDevCtx.avg); — End diff – Actually, datum.asFloat8() - StdDevCtx.avg in line 52 is not equal to delta because StdDevCtx.avg is updated after delta is calculated.
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user jihoonson commented on a diff in the pull request:

      https://github.com/apache/tajo/pull/427#discussion_r27366305

      — Diff: tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDev.java —
      @@ -0,0 +1,94 @@
      +/**
      + * 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.tajo.engine.function.builtin;
      +
      +import org.apache.tajo.catalog.CatalogUtil;
      +import org.apache.tajo.catalog.Column;
      +import org.apache.tajo.common.TajoDataTypes.DataType;
      +import org.apache.tajo.common.TajoDataTypes.Type;
      +import org.apache.tajo.datum.Datum;
      +import org.apache.tajo.datum.NullDatum;
      +import org.apache.tajo.datum.ProtobufDatum;
      +import org.apache.tajo.plan.function.AggFunction;
      +import org.apache.tajo.plan.function.FunctionContext;
      +import org.apache.tajo.storage.Tuple;
      +
      +import static org.apache.tajo.InternalTypes.StdDevProto;
      +
      +public abstract class StdDev extends AggFunction<Datum> {
      +
      + public StdDev(Column[] definedArgs)

      { + super(definedArgs); + }

      +
      + public StdDevContext newContext()

      { + return new StdDevContext(); + }

      +
      + @Override
      + public void eval(FunctionContext ctx, Tuple params) {
      + StdDevContext StdDevCtx = (StdDevContext) ctx;
      + Datum datum = params.get(0);
      + if (datum.isNotNull()) {
      + double delta = datum.asFloat8() - StdDevCtx.avg;
      + StdDevCtx.count++;
      + StdDevCtx.avg += delta/StdDevCtx.count;
      + StdDevCtx.squareSumOfDiff += delta * (datum.asFloat8() - StdDevCtx.avg);
      — End diff –

      ```datum.asFloat8() - StdDevCtx.avg``` can be replaced with ```delta```.

      Show
      githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on a diff in the pull request: https://github.com/apache/tajo/pull/427#discussion_r27366305 — Diff: tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDev.java — @@ -0,0 +1,94 @@ +/** + * 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.tajo.engine.function.builtin; + +import org.apache.tajo.catalog.CatalogUtil; +import org.apache.tajo.catalog.Column; +import org.apache.tajo.common.TajoDataTypes.DataType; +import org.apache.tajo.common.TajoDataTypes.Type; +import org.apache.tajo.datum.Datum; +import org.apache.tajo.datum.NullDatum; +import org.apache.tajo.datum.ProtobufDatum; +import org.apache.tajo.plan.function.AggFunction; +import org.apache.tajo.plan.function.FunctionContext; +import org.apache.tajo.storage.Tuple; + +import static org.apache.tajo.InternalTypes.StdDevProto; + +public abstract class StdDev extends AggFunction<Datum> { + + public StdDev(Column[] definedArgs) { + super(definedArgs); + } + + public StdDevContext newContext() { + return new StdDevContext(); + } + + @Override + public void eval(FunctionContext ctx, Tuple params) { + StdDevContext StdDevCtx = (StdDevContext) ctx; + Datum datum = params.get(0); + if (datum.isNotNull()) { + double delta = datum.asFloat8() - StdDevCtx.avg; + StdDevCtx.count++; + StdDevCtx.avg += delta/StdDevCtx.count; + StdDevCtx.squareSumOfDiff += delta * (datum.asFloat8() - StdDevCtx.avg); — End diff – ```datum.asFloat8() - StdDevCtx.avg``` can be replaced with ```delta```.
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user jihoonson commented on the pull request:

      https://github.com/apache/tajo/pull/427#issuecomment-86777274

      I'll review today.

      Show
      githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/427#issuecomment-86777274 I'll review today.
      Hide
      githubbot ASF GitHub Bot added a comment -

      GitHub user sirpkt opened a pull request:

      https://github.com/apache/tajo/pull/427

      TAJO-921: Add STDDEV_SAMP and STDDEV_POP window functions

      Implementing STDDEV_SAMP() and STDDEV_POP() as aggregation funciton.

      • Calculation is based on the method first proposed by B. P. Welford in 1962 and described by Donald Knuth in Art of Computer Programming, Volume 2: Seminumerical Algorithms.
        (You can also check http://en.wikipedia.org/wiki/Algorithms_for_calculating_variance#Online_algorithm for basic explanation)
      • I also changes names of test methods for window functions (for example, from rowNumber1 to testRowNumber1)

      I checked 'mvn clean install' passed with this patch.

      You can merge this pull request into a Git repository by running:

      $ git pull https://github.com/sirpkt/tajo TAJO-921

      Alternatively you can review and apply these changes as the patch at:

      https://github.com/apache/tajo/pull/427.patch

      To close this pull request, make a commit to your master/trunk branch
      with (at least) the following in the commit message:

      This closes #427


      commit e8f8ca78b30b8efb5eb8418b3d811a7053cd24f5
      Author: Keuntae Park <sirpkt@apache.org>
      Date: 2015-03-17T01:58:58Z

      Implement aggregation functions STDDEV_SAMP and STDDEV_POP

      commit 406f3113c796d79c6f6888b68eac00a8ee0f5039
      Author: Keuntae Park <sirpkt@apache.org>
      Date: 2015-03-17T04:54:09Z

      Merge remote-tracking branch 'upstream/master' into TAJO-921


      Show
      githubbot ASF GitHub Bot added a comment - GitHub user sirpkt opened a pull request: https://github.com/apache/tajo/pull/427 TAJO-921 : Add STDDEV_SAMP and STDDEV_POP window functions Implementing STDDEV_SAMP() and STDDEV_POP() as aggregation funciton. Calculation is based on the method first proposed by B. P. Welford in 1962 and described by Donald Knuth in Art of Computer Programming, Volume 2: Seminumerical Algorithms. (You can also check http://en.wikipedia.org/wiki/Algorithms_for_calculating_variance#Online_algorithm for basic explanation) I also changes names of test methods for window functions (for example, from rowNumber1 to testRowNumber1) I checked 'mvn clean install' passed with this patch. You can merge this pull request into a Git repository by running: $ git pull https://github.com/sirpkt/tajo TAJO-921 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/427.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #427 commit e8f8ca78b30b8efb5eb8418b3d811a7053cd24f5 Author: Keuntae Park <sirpkt@apache.org> Date: 2015-03-17T01:58:58Z Implement aggregation functions STDDEV_SAMP and STDDEV_POP commit 406f3113c796d79c6f6888b68eac00a8ee0f5039 Author: Keuntae Park <sirpkt@apache.org> Date: 2015-03-17T04:54:09Z Merge remote-tracking branch 'upstream/master' into TAJO-921
      Hide
      sirpkt Keuntae Park added a comment -

      If no one starts, I'll take this issue.

      Show
      sirpkt Keuntae Park added a comment - If no one starts, I'll take this issue.
      Hide
      dongjoon Dongjoon Hyun added a comment -

      +1. This functions are needed in many cases including TPC-DS query 17 and 39.

      Show
      dongjoon Dongjoon Hyun added a comment - +1. This functions are needed in many cases including TPC-DS query 17 and 39.

        People

        • Assignee:
          sirpkt Keuntae Park
          Reporter:
          hyunsik Hyunsik Choi
        • Votes:
          0 Vote for this issue
          Watchers:
          4 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development