Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.11.0
    • Component/s: Function/UDF
    • Labels:
      None
    1. TAJO-923.Hyun.150416.0.patch.txt
      24 kB
      Dongjoon Hyun
    2. TAJO-923.Hyun.150417.0.patch.txt
      34 kB
      Dongjoon Hyun
    3. TAJO-923.patch
      28 kB
      Dongjoon Hyun

      Activity

      Hide
      hudson Hudson added a comment -

      SUCCESS: Integrated in Tajo-master-build #672 (See https://builds.apache.org/job/Tajo-master-build/672/)
      TAJO-923: Add VAR_SAMP and VAR_POP window functions. (jihoonson: rev 14a1e536c2848dbb9e1a714076d833a7ff1f5640)

      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampDouble.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSamp.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSamp.java
      • CHANGES
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampInt.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopInt.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopFloat.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPop.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampFloat.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPop.java
      • tajo-core/src/test/java/org/apache/tajo/engine/function/TestBuiltinFunctions.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDev.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampLong.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopLong.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopDouble.java
      • tajo-core/src/main/proto/InternalTypes.proto
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/Variance.java
      Show
      hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #672 (See https://builds.apache.org/job/Tajo-master-build/672/ ) TAJO-923 : Add VAR_SAMP and VAR_POP window functions. (jihoonson: rev 14a1e536c2848dbb9e1a714076d833a7ff1f5640) tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampDouble.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSamp.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSamp.java CHANGES tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampInt.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopInt.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopFloat.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPop.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampFloat.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPop.java tajo-core/src/test/java/org/apache/tajo/engine/function/TestBuiltinFunctions.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDev.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampLong.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopLong.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopDouble.java tajo-core/src/main/proto/InternalTypes.proto tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/Variance.java
      Hide
      hudson Hudson added a comment -

      FAILURE: Integrated in Tajo-master-CODEGEN-build #310 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/310/)
      TAJO-923: Add VAR_SAMP and VAR_POP window functions. (jihoonson: rev 14a1e536c2848dbb9e1a714076d833a7ff1f5640)

      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/Variance.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSamp.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSamp.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampInt.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampDouble.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampLong.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPop.java
      • CHANGES
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopLong.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDev.java
      • tajo-core/src/test/java/org/apache/tajo/engine/function/TestBuiltinFunctions.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopDouble.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopFloat.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPop.java
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopInt.java
      • tajo-core/src/main/proto/InternalTypes.proto
      • tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampFloat.java
      Show
      hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #310 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/310/ ) TAJO-923 : Add VAR_SAMP and VAR_POP window functions. (jihoonson: rev 14a1e536c2848dbb9e1a714076d833a7ff1f5640) tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/Variance.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSamp.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevSamp.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampInt.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampDouble.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampLong.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPop.java CHANGES tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopLong.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDev.java tajo-core/src/test/java/org/apache/tajo/engine/function/TestBuiltinFunctions.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopDouble.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopFloat.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/StdDevPop.java tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPopInt.java tajo-core/src/main/proto/InternalTypes.proto tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarSampFloat.java
      Hide
      jihoonson Jihoon Son added a comment -

      Committed to the master branch.
      Thanks!

      Show
      jihoonson Jihoon Son added a comment - Committed to the master branch. Thanks!
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user asfgit closed the pull request at:

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

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

      Github user jihoonson commented on the pull request:

      https://github.com/apache/tajo/pull/458#issuecomment-94001545

      +1 thanks for your work.

      Show
      githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/458#issuecomment-94001545 +1 thanks for your work.
      Hide
      tajoqa Tajo QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12726134/TAJO-923.Hyun.150417.0.patch.txt
      against master revision release-0.9.0-rc0-258-gd2a4f9b.

      +1 @author. The patch does not contain any @author tags.

      +1 tests included. The patch appears to include 1 new or modified test files.

      +1 javac. The applied patch does not increase the total number of javac compiler warnings.

      +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

      +1 checkstyle. The patch generated 0 code style errors.

      -1 findbugs. The patch appears to introduce 18 new Findbugs (version 2.0.3) warnings.

      +1 release audit. The applied patch does not increase the total number of release audit warnings.

      +1 core tests. The patch passed unit tests in tajo-core.

      Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/739//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/739//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html
      Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/739//console

      This message is automatically generated.

      Show
      tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12726134/TAJO-923.Hyun.150417.0.patch.txt against master revision release-0.9.0-rc0-258-gd2a4f9b. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 18 new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-core. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/739//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/739//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/739//console This message is automatically generated.
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user dongjoon-hyun commented on a diff in the pull request:

      https://github.com/apache/tajo/pull/458#discussion_r28588037

      — Diff: tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPop.java —
      @@ -0,0 +1,42 @@
      +/**
      + * 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.Column;
      +import org.apache.tajo.datum.Datum;
      +import org.apache.tajo.datum.DatumFactory;
      +import org.apache.tajo.datum.NullDatum;
      +import org.apache.tajo.plan.function.FunctionContext;
      +
      +public abstract class VarPop extends StdDev {
      — End diff –

      @jihoonson , I changed the code according to your comments. Thank you always.

      Show
      githubbot ASF GitHub Bot added a comment - Github user dongjoon-hyun commented on a diff in the pull request: https://github.com/apache/tajo/pull/458#discussion_r28588037 — Diff: tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPop.java — @@ -0,0 +1,42 @@ +/** + * 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.Column; +import org.apache.tajo.datum.Datum; +import org.apache.tajo.datum.DatumFactory; +import org.apache.tajo.datum.NullDatum; +import org.apache.tajo.plan.function.FunctionContext; + +public abstract class VarPop extends StdDev { — End diff – @jihoonson , I changed the code according to your comments. Thank you always.
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user dongjoon-hyun commented on a diff in the pull request:

      https://github.com/apache/tajo/pull/458#discussion_r28584422

      — Diff: tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPop.java —
      @@ -0,0 +1,42 @@
      +/**
      + * 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.Column;
      +import org.apache.tajo.datum.Datum;
      +import org.apache.tajo.datum.DatumFactory;
      +import org.apache.tajo.datum.NullDatum;
      +import org.apache.tajo.plan.function.FunctionContext;
      +
      +public abstract class VarPop extends StdDev {
      — End diff –

      It sounds good to me, too. I agree that `Variance` looks more general to cover all VarPop/VarSamp/StdDevPop/StdDevSamp functions. Then, `StdDevContext` should be changed into `VarianceContext` accordingly.

      Show
      githubbot ASF GitHub Bot added a comment - Github user dongjoon-hyun commented on a diff in the pull request: https://github.com/apache/tajo/pull/458#discussion_r28584422 — Diff: tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPop.java — @@ -0,0 +1,42 @@ +/** + * 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.Column; +import org.apache.tajo.datum.Datum; +import org.apache.tajo.datum.DatumFactory; +import org.apache.tajo.datum.NullDatum; +import org.apache.tajo.plan.function.FunctionContext; + +public abstract class VarPop extends StdDev { — End diff – It sounds good to me, too. I agree that `Variance` looks more general to cover all VarPop/VarSamp/StdDevPop/StdDevSamp functions. Then, `StdDevContext` should be changed into `VarianceContext` accordingly.
      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/458#discussion_r28582879

      — Diff: tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPop.java —
      @@ -0,0 +1,42 @@
      +/**
      + * 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.Column;
      +import org.apache.tajo.datum.Datum;
      +import org.apache.tajo.datum.DatumFactory;
      +import org.apache.tajo.datum.NullDatum;
      +import org.apache.tajo.plan.function.FunctionContext;
      +
      +public abstract class VarPop extends StdDev {
      — End diff –

      I think that we need to change ```StdDev``` to more general name.
      How about ```Variance```? Or, do you have any good idea?

      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/458#discussion_r28582879 — Diff: tajo-core/src/main/java/org/apache/tajo/engine/function/builtin/VarPop.java — @@ -0,0 +1,42 @@ +/** + * 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.Column; +import org.apache.tajo.datum.Datum; +import org.apache.tajo.datum.DatumFactory; +import org.apache.tajo.datum.NullDatum; +import org.apache.tajo.plan.function.FunctionContext; + +public abstract class VarPop extends StdDev { — End diff – I think that we need to change ```StdDev``` to more general name. How about ```Variance```? Or, do you have any good idea?
      Hide
      tajoqa Tajo QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12725767/TAJO-923.Hyun.150416.0.patch.txt
      against master revision release-0.9.0-rc0-254-gc42d4b3.

      +1 @author. The patch does not contain any @author tags.

      +1 tests included. The patch appears to include 1 new or modified test files.

      +1 javac. The applied patch does not increase the total number of javac compiler warnings.

      +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

      +1 checkstyle. The patch generated 0 code style errors.

      -1 findbugs. The patch appears to introduce 17 new Findbugs (version 2.0.3) warnings.

      +1 release audit. The applied patch does not increase the total number of release audit warnings.

      +1 core tests. The patch passed unit tests in tajo-core.

      Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/732//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/732//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html
      Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/732//console

      This message is automatically generated.

      Show
      tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12725767/TAJO-923.Hyun.150416.0.patch.txt against master revision release-0.9.0-rc0-254-gc42d4b3. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 17 new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-core. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/732//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/732//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/732//console This message is automatically generated.
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user dongjoon-hyun commented on the pull request:

      https://github.com/apache/tajo/pull/458#issuecomment-93621592

      Hi, @jihoonson . I removed the following redundancy. Now, VarSamp and VarPop use StdDevContext.

      • ProtocolBuffer message for VarSamp/VarPop
      • Var.java file
      Show
      githubbot ASF GitHub Bot added a comment - Github user dongjoon-hyun commented on the pull request: https://github.com/apache/tajo/pull/458#issuecomment-93621592 Hi, @jihoonson . I removed the following redundancy. Now, VarSamp and VarPop use StdDevContext. ProtocolBuffer message for VarSamp/VarPop Var.java file
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user jihoonson commented on the pull request:

      https://github.com/apache/tajo/pull/458#issuecomment-92156262

      Thanks for your rapid reply.
      Yes, I also think that will be a good way.

      Show
      githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/458#issuecomment-92156262 Thanks for your rapid reply. Yes, I also think that will be a good way.
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user dongjoon-hyun commented on the pull request:

      https://github.com/apache/tajo/pull/458#issuecomment-92155775

      @jihoonson , I knew that 20 days ago. But, there was no way to implement variance at that time, since TAJO-921 is not committed yet. As I mentioned my first comment, we need to change.

      Here, I need your guide how to reduce the redundancy. Is it proper to compute STD_DEV first and square that?

      Show
      githubbot ASF GitHub Bot added a comment - Github user dongjoon-hyun commented on the pull request: https://github.com/apache/tajo/pull/458#issuecomment-92155775 @jihoonson , I knew that 20 days ago. But, there was no way to implement variance at that time, since TAJO-921 is not committed yet. As I mentioned my first comment, we need to change. Here, I need your guide how to reduce the redundancy. Is it proper to compute STD_DEV first and square that?
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user jihoonson commented on the pull request:

      https://github.com/apache/tajo/pull/458#issuecomment-92151813

      No offense. Sorry for late review.

      Show
      githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/458#issuecomment-92151813 No offense. Sorry for late review.
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user jihoonson commented on the pull request:

      https://github.com/apache/tajo/pull/458#issuecomment-92151482

      The only difference between this patch and TAJO-921 is the absence of square root. Why should we maintan mostly duplicated codes separately?

      Show
      githubbot ASF GitHub Bot added a comment - Github user jihoonson commented on the pull request: https://github.com/apache/tajo/pull/458#issuecomment-92151482 The only difference between this patch and TAJO-921 is the absence of square root. Why should we maintan mostly duplicated codes separately?
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user jihoonson commented on the pull request:

      https://github.com/apache/tajo/pull/458#issuecomment-89803831

      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/458#issuecomment-89803831 I'll review today.
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user dongjoon-hyun commented on the pull request:

      https://github.com/apache/tajo/pull/458#issuecomment-88881774

      Rebased.

      Show
      githubbot ASF GitHub Bot added a comment - Github user dongjoon-hyun commented on the pull request: https://github.com/apache/tajo/pull/458#issuecomment-88881774 Rebased.
      Hide
      githubbot ASF GitHub Bot added a comment -

      Github user dongjoon-hyun commented on the pull request:

      https://github.com/apache/tajo/pull/458#issuecomment-87956772

      Rebased.

      Show
      githubbot ASF GitHub Bot added a comment - Github user dongjoon-hyun commented on the pull request: https://github.com/apache/tajo/pull/458#issuecomment-87956772 Rebased.
      Hide
      tajoqa Tajo QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12706810/TAJO-923.patch
      against master revision release-0.9.0-rc0-214-g8d0146b.

      +1 @author. The patch does not contain any @author tags.

      +1 tests included. The patch appears to include 1 new or modified test files.

      +1 javac. The applied patch does not increase the total number of javac compiler warnings.

      +1 javadoc. The applied patch does not increase the total number of javadoc warnings.

      +1 checkstyle. The patch generated 0 code style errors.

      -1 findbugs. The patch appears to introduce 2 new Findbugs (version 2.0.3) warnings.

      +1 release audit. The applied patch does not increase the total number of release audit warnings.

      +1 core tests. The patch passed unit tests in tajo-core.

      Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/646//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/646//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html
      Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/646//console

      This message is automatically generated.

      Show
      tajoqa Tajo QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12706810/TAJO-923.patch against master revision release-0.9.0-rc0-214-g8d0146b. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The applied patch does not increase the total number of javadoc warnings. +1 checkstyle. The patch generated 0 code style errors. -1 findbugs. The patch appears to introduce 2 new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in tajo-core. Test results: https://builds.apache.org/job/PreCommit-TAJO-Build/646//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-TAJO-Build/646//artifact/incubator-tajo/patchprocess/newPatchFindbugsWarningstajo-core.html Console output: https://builds.apache.org/job/PreCommit-TAJO-Build/646//console This message is automatically generated.
      Hide
      githubbot ASF GitHub Bot added a comment -

      GitHub user dongjoon-hyun opened a pull request:

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

      TAJO-923: Add VAR_SAMP and VAR_POP window functions

      I coded in the same way in https://github.com/apache/tajo/pull/427 (TAJO-921)
      If needed, after merging pull-request 427, I will do refactoring.

      This code is passed the following command.
      ```
      mvn clean install -Pparallel-test,hcatalog-0.12.0 -DLOG_LEVEL=ERROR -Dmaven.fork.count=2 &> TAJO-923.travis.ERROR.txt
      ```
      The result file is https://app.box.com/s/clnx9d2bisfhbh2upgo3q6g7bdx7aic5

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

      $ git pull https://github.com/dongjoon-hyun/tajo TAJO-923

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

      https://github.com/apache/tajo/pull/458.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 #458


      commit 0b6fbbc2a04ffa870b835c5b35b779a1094348c3
      Author: Dongjoon Hyun <dongjoon@apache.org>
      Date: 2015-03-24T00:02:51Z

      TAJO-923: Add VAR_SAMP and VAR_POP window functions


      Show
      githubbot ASF GitHub Bot added a comment - GitHub user dongjoon-hyun opened a pull request: https://github.com/apache/tajo/pull/458 TAJO-923 : Add VAR_SAMP and VAR_POP window functions I coded in the same way in https://github.com/apache/tajo/pull/427 ( TAJO-921 ) If needed, after merging pull-request 427, I will do refactoring. This code is passed the following command. ``` mvn clean install -Pparallel-test,hcatalog-0.12.0 -DLOG_LEVEL=ERROR -Dmaven.fork.count=2 &> TAJO-923 .travis.ERROR.txt ``` The result file is https://app.box.com/s/clnx9d2bisfhbh2upgo3q6g7bdx7aic5 You can merge this pull request into a Git repository by running: $ git pull https://github.com/dongjoon-hyun/tajo TAJO-923 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/458.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 #458 commit 0b6fbbc2a04ffa870b835c5b35b779a1094348c3 Author: Dongjoon Hyun <dongjoon@apache.org> Date: 2015-03-24T00:02:51Z TAJO-923 : Add VAR_SAMP and VAR_POP window functions

        People

        • Assignee:
          dongjoon Dongjoon Hyun
          Reporter:
          hyunsik Hyunsik Choi
        • Votes:
          0 Vote for this issue
          Watchers:
          3 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development