Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.12.0, 0.11.1
    • Component/s: Client
    • Labels:
      None

      Description

      Java TimezoneId doesn't handle below case

      Asia/Seoul is not same ASIA/Seoul

      so. it cann't find proper timezone ID

        Activity

        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user charsyam opened a pull request:

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

        TAJO-1993 Table Timezone doesn't work when Timezone is not exactly same.

        Auto Converted

        ASIA/Seoul -> Asia/Seoul
        Asia/seoul -> Asia/Seoul

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

        $ git pull https://github.com/charsyam/tajo feature/TAJO-1993

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

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


        commit 1fac21a2e453a46d25869744df14d38f1be4eb0e
        Author: charsyam <charsyam@charsyamui-macbook-pro.local>
        Date: 2015-11-28T04:33:07Z

        converted case sensative timezone


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user charsyam opened a pull request: https://github.com/apache/tajo/pull/888 TAJO-1993 Table Timezone doesn't work when Timezone is not exactly same. Auto Converted ASIA/Seoul -> Asia/Seoul Asia/seoul -> Asia/Seoul You can merge this pull request into a Git repository by running: $ git pull https://github.com/charsyam/tajo feature/ TAJO-1993 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/888.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 #888 commit 1fac21a2e453a46d25869744df14d38f1be4eb0e Author: charsyam <charsyam@charsyamui-macbook-pro.local> Date: 2015-11-28T04:33:07Z converted case sensative timezone
        Hide
        githubbot ASF GitHub Bot added a comment -

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

        https://github.com/apache/tajo/pull/888#discussion_r46362287

        — Diff: tajo-core/src/main/java/org/apache/tajo/util/TimeZoneUtil.java —
        @@ -0,0 +1,64 @@
        +/**
        + * 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.util;
        +
        +import com.facebook.presto.hive.shaded.com.google.common.collect.Maps;
        — End diff –

        It should be ``com.google.common.collect.Maps`` instead of presto one.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on a diff in the pull request: https://github.com/apache/tajo/pull/888#discussion_r46362287 — Diff: tajo-core/src/main/java/org/apache/tajo/util/TimeZoneUtil.java — @@ -0,0 +1,64 @@ +/** + * 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.util; + +import com.facebook.presto.hive.shaded.com.google.common.collect.Maps; — End diff – It should be ``com.google.common.collect.Maps`` instead of presto one.
        Hide
        githubbot ASF GitHub Bot added a comment -

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

        https://github.com/apache/tajo/pull/888#discussion_r46362467

        — Diff: tajo-core/src/main/java/org/apache/tajo/util/TimeZoneUtil.java —
        @@ -0,0 +1,64 @@
        +/**
        + * 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.util;
        +
        +import com.facebook.presto.hive.shaded.com.google.common.collect.Maps;
        +import org.apache.tajo.SessionVars;
        +import org.apache.tajo.storage.StorageConstants;
        +
        +import java.util.Map;
        +
        +public class TimeZoneUtil {
        + static Map<String, String> timeZoneIdMap;
        — End diff –

        It can be an ImmutableHashMap. Also, we can mark ``final`` for this member variable.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on a diff in the pull request: https://github.com/apache/tajo/pull/888#discussion_r46362467 — Diff: tajo-core/src/main/java/org/apache/tajo/util/TimeZoneUtil.java — @@ -0,0 +1,64 @@ +/** + * 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.util; + +import com.facebook.presto.hive.shaded.com.google.common.collect.Maps; +import org.apache.tajo.SessionVars; +import org.apache.tajo.storage.StorageConstants; + +import java.util.Map; + +public class TimeZoneUtil { + static Map<String, String> timeZoneIdMap; — End diff – It can be an ImmutableHashMap. Also, we can mark ``final`` for this member variable.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/888#issuecomment-161141857

        The patch looks good to me. I leave some trivial comments.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/888#issuecomment-161141857 The patch looks good to me. I leave some trivial comments.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user charsyam commented on the pull request:

        https://github.com/apache/tajo/pull/888#issuecomment-161343415

        @hyunsik I applied your review Thanks.

        Show
        githubbot ASF GitHub Bot added a comment - Github user charsyam commented on the pull request: https://github.com/apache/tajo/pull/888#issuecomment-161343415 @hyunsik I applied your review Thanks.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user charsyam commented on the pull request:

        https://github.com/apache/tajo/pull/888#issuecomment-161683587

        @hyunsik Thanks for your review
        I applied it

        Show
        githubbot ASF GitHub Bot added a comment - Github user charsyam commented on the pull request: https://github.com/apache/tajo/pull/888#issuecomment-161683587 @hyunsik Thanks for your review I applied it
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user hyunsik commented on the pull request:

        https://github.com/apache/tajo/pull/888#issuecomment-161915189

        +1

        One test is failed, but it is not related to your change. The patch looks good to me.

        Show
        githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/888#issuecomment-161915189 +1 One test is failed, but it is not related to your change. The patch looks good to me.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

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

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

        FAILURE: Integrated in Tajo-master-CODEGEN-build #620 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/620/)
        TAJO-1993: Table Timezone doesn't work when Timezone is not exactly (charsyam: rev 4b53643d47f9afbd8ec52db9cb7d7fae8b03875d)

        • tajo-core/src/main/java/org/apache/tajo/util/TimeZoneUtil.java
        • tajo-core-tests/src/test/java/org/apache/tajo/util/TestTimeZoneUtil.java
        • tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java
        • CHANGES
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #620 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/620/ ) TAJO-1993 : Table Timezone doesn't work when Timezone is not exactly (charsyam: rev 4b53643d47f9afbd8ec52db9cb7d7fae8b03875d) tajo-core/src/main/java/org/apache/tajo/util/TimeZoneUtil.java tajo-core-tests/src/test/java/org/apache/tajo/util/TestTimeZoneUtil.java tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java CHANGES
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-master-build #1014 (See https://builds.apache.org/job/Tajo-master-build/1014/)
        TAJO-1993: Table Timezone doesn't work when Timezone is not exactly (charsyam: rev 4b53643d47f9afbd8ec52db9cb7d7fae8b03875d)

        • tajo-core-tests/src/test/java/org/apache/tajo/util/TestTimeZoneUtil.java
        • tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java
        • CHANGES
        • tajo-core/src/main/java/org/apache/tajo/util/TimeZoneUtil.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #1014 (See https://builds.apache.org/job/Tajo-master-build/1014/ ) TAJO-1993 : Table Timezone doesn't work when Timezone is not exactly (charsyam: rev 4b53643d47f9afbd8ec52db9cb7d7fae8b03875d) tajo-core-tests/src/test/java/org/apache/tajo/util/TestTimeZoneUtil.java tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java CHANGES tajo-core/src/main/java/org/apache/tajo/util/TimeZoneUtil.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Tajo-0.11.1-build #131 (See https://builds.apache.org/job/Tajo-0.11.1-build/131/)
        TAJO-1993: Table Timezone doesn't work when Timezone is not exactly (charsyam: rev 20d850121b2872290fcab633edd9dc0ad913a0aa)

        • tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java
        • tajo-core/src/main/java/org/apache/tajo/util/TimeZoneUtil.java
        • tajo-core-tests/src/test/java/org/apache/tajo/util/TestTimeZoneUtil.java
        • CHANGES
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Tajo-0.11.1-build #131 (See https://builds.apache.org/job/Tajo-0.11.1-build/131/ ) TAJO-1993 : Table Timezone doesn't work when Timezone is not exactly (charsyam: rev 20d850121b2872290fcab633edd9dc0ad913a0aa) tajo-core/src/main/java/org/apache/tajo/parser/sql/SQLAnalyzer.java tajo-core/src/main/java/org/apache/tajo/util/TimeZoneUtil.java tajo-core-tests/src/test/java/org/apache/tajo/util/TestTimeZoneUtil.java CHANGES

          People

          • Assignee:
            charsyam DaeMyung Kang
            Reporter:
            charsyam DaeMyung Kang
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development