Repository: phoenix Updated Branches: refs/heads/4.3 1e48eacde -> 66411e775
Revert "PHOENIX-1722 Speedup CONVERT_TZ function (Vaclav Loffelmann)" Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/66411e77 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/66411e77 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/66411e77 Branch: refs/heads/4.3 Commit: 66411e7759aae9768439a591a64f1d180d94992e Parents: 1e48eac Author: Samarth <[email protected]> Authored: Fri Mar 27 17:34:38 2015 -0700 Committer: Samarth <[email protected]> Committed: Fri Mar 27 17:34:38 2015 -0700 ---------------------------------------------------------------------- .../end2end/ConvertTimezoneFunctionIT.java | 24 +----- .../apache/phoenix/cache/JodaTimezoneCache.java | 84 -------------------- .../function/ConvertTimezoneFunction.java | 38 ++++++--- .../function/TimezoneOffsetFunction.java | 25 ++++-- .../phoenix/cache/JodaTimezoneCacheTest.java | 51 ------------ pom.xml | 2 +- 6 files changed, 51 insertions(+), 173 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/66411e77/phoenix-core/src/it/java/org/apache/phoenix/end2end/ConvertTimezoneFunctionIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ConvertTimezoneFunctionIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ConvertTimezoneFunctionIT.java index f415dc6..d89a03b 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ConvertTimezoneFunctionIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ConvertTimezoneFunctionIT.java @@ -23,10 +23,8 @@ import java.sql.Connection; import java.sql.DriverManager; import java.sql.ResultSet; import java.sql.SQLException; -import java.sql.Statement; import org.apache.phoenix.exception.SQLExceptionCode; -import static org.junit.Assert.assertFalse; import org.junit.Test; /** @@ -131,7 +129,7 @@ public class ConvertTimezoneFunctionIT extends BaseHBaseManagedTimeIT { try { ResultSet rs = conn.createStatement().executeQuery( "SELECT k1, dates, CONVERT_TZ(dates, 'UNKNOWN_TIMEZONE', 'America/Adak') FROM TIMEZONE_OFFSET_TEST"); - + rs.next(); rs.getDate(3).getTime(); fail(); @@ -139,24 +137,4 @@ public class ConvertTimezoneFunctionIT extends BaseHBaseManagedTimeIT { assertEquals(SQLExceptionCode.ILLEGAL_DATA.getErrorCode(), e.getErrorCode()); } } - - @Test - public void testConvertMultipleRecords() throws Exception { - Connection conn = DriverManager.getConnection(getUrl()); - String ddl = "CREATE TABLE IF NOT EXISTS TIMEZONE_OFFSET_TEST (k1 INTEGER NOT NULL, dates DATE CONSTRAINT pk PRIMARY KEY (k1))"; - Statement stmt = conn.createStatement(); - stmt.execute(ddl); - stmt.execute("UPSERT INTO TIMEZONE_OFFSET_TEST (k1, dates) VALUES (1, TO_DATE('2014-03-01 00:00:00'))"); - stmt.execute("UPSERT INTO TIMEZONE_OFFSET_TEST (k1, dates) VALUES (2, TO_DATE('2014-03-01 00:00:00'))"); - conn.commit(); - - ResultSet rs = stmt.executeQuery( - "SELECT k1, dates, CONVERT_TZ(dates, 'UTC', 'America/Adak') FROM TIMEZONE_OFFSET_TEST"); - - assertTrue(rs.next()); - assertEquals(1393596000000L, rs.getDate(3).getTime()); //Fri, 28 Feb 2014 14:00:00 - assertTrue(rs.next()); - assertEquals(1393596000000L, rs.getDate(3).getTime()); //Fri, 28 Feb 2014 14:00:00 - assertFalse(rs.next()); - } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/66411e77/phoenix-core/src/main/java/org/apache/phoenix/cache/JodaTimezoneCache.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/cache/JodaTimezoneCache.java b/phoenix-core/src/main/java/org/apache/phoenix/cache/JodaTimezoneCache.java deleted file mode 100644 index 54904d7..0000000 --- a/phoenix-core/src/main/java/org/apache/phoenix/cache/JodaTimezoneCache.java +++ /dev/null @@ -1,84 +0,0 @@ -/* - * Copyright 2015 Apache Software Foundation. - * - * Licensed 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.phoenix.cache; - -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; -import com.google.common.util.concurrent.UncheckedExecutionException; -import java.nio.ByteBuffer; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import org.apache.hadoop.hbase.io.ImmutableBytesWritable; -import org.apache.hadoop.hbase.util.Bytes; -import org.apache.phoenix.schema.IllegalDataException; -import org.joda.time.DateTimeZone; - -public class JodaTimezoneCache { - - public static final int CACHE_EXPRIRE_TIME_MINUTES = 10; - private static final LoadingCache<ByteBuffer, DateTimeZone> cachedJodaTimeZones = createTimezoneCache(); - - /** - * Returns joda's DateTimeZone instance from cache or create new instance and cache it. - * - * @param timezoneId Timezone Id as accepted by {@code DateTimeZone.forID()}. E.g. Europe/Isle_of_Man - * @return joda's DateTimeZone instance - * @throws IllegalDataException if unknown timezone id is passed - */ - public static DateTimeZone getInstance(ByteBuffer timezoneId) { - try { - return cachedJodaTimeZones.get(timezoneId); - } catch (ExecutionException ex) { - throw new IllegalDataException(ex); - } catch (UncheckedExecutionException e) { - throw new IllegalDataException("Unknown timezone " + Bytes.toString(timezoneId.array())); - } - } - - /** - * Returns joda's DateTimeZone instance from cache or create new instance and cache it. - * - * @param timezoneId Timezone Id as accepted by {@code DateTimeZone.forID()}. E.g. Europe/Isle_of_Man - * @return joda's DateTimeZone instance - * @throws IllegalDataException if unknown timezone id is passed - */ - public static DateTimeZone getInstance(ImmutableBytesWritable timezoneId) { - return getInstance(ByteBuffer.wrap(timezoneId.copyBytes())); - } - - /** - * Returns joda's DateTimeZone instance from cache or create new instance and cache it. - * - * @param timezoneId Timezone Id as accepted by {@code DateTimeZone.forID()}. E.g. Europe/Isle_of_Man - * @return joda's DateTimeZone instance - * @throws IllegalDataException if unknown timezone id is passed - */ - public static DateTimeZone getInstance(String timezoneId) { - return getInstance(ByteBuffer.wrap(Bytes.toBytes(timezoneId))); - } - - private static LoadingCache<ByteBuffer, DateTimeZone> createTimezoneCache() { - return CacheBuilder.newBuilder().expireAfterAccess(CACHE_EXPRIRE_TIME_MINUTES, TimeUnit.MINUTES).build(new CacheLoader<ByteBuffer, DateTimeZone>() { - - @Override - public DateTimeZone load(ByteBuffer timezone) throws Exception { - return DateTimeZone.forID(Bytes.toString(timezone.array())); - } - }); - } - -} http://git-wip-us.apache.org/repos/asf/phoenix/blob/66411e77/phoenix-core/src/main/java/org/apache/phoenix/expression/function/ConvertTimezoneFunction.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/ConvertTimezoneFunction.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/ConvertTimezoneFunction.java index 3ea47a6..dcde31f 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/ConvertTimezoneFunction.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/ConvertTimezoneFunction.java @@ -15,17 +15,21 @@ */ package org.apache.phoenix.expression.function; +import java.sql.Date; import java.sql.SQLException; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.TimeZone; import org.apache.hadoop.hbase.io.ImmutableBytesWritable; -import org.apache.phoenix.cache.JodaTimezoneCache; +import org.apache.hadoop.hbase.util.Bytes; import org.apache.phoenix.expression.Expression; import org.apache.phoenix.parse.FunctionParseNode; +import org.apache.phoenix.schema.IllegalDataException; import org.apache.phoenix.schema.types.PDataType; import org.apache.phoenix.schema.types.PDate; import org.apache.phoenix.schema.types.PVarchar; import org.apache.phoenix.schema.tuple.Tuple; -import org.joda.time.DateTimeZone; /** * Build in function CONVERT_TZ(date, 'timezone_from', 'timezone_to). Convert date from one timezone to @@ -39,6 +43,7 @@ import org.joda.time.DateTimeZone; public class ConvertTimezoneFunction extends ScalarFunction { public static final String NAME = "CONVERT_TZ"; + private final Map<String, TimeZone> cachedTimeZones = new HashMap<String, TimeZone>(); public ConvertTimezoneFunction() { } @@ -57,25 +62,40 @@ public class ConvertTimezoneFunction extends ScalarFunction { if (!children.get(0).evaluate(tuple, ptr)) { return false; } - long date = PDate.INSTANCE.getCodec().decodeLong(ptr, children.get(0).getSortOrder()); + + Date dateo = (Date) PDate.INSTANCE.toObject(ptr, children.get(0).getSortOrder()); + Long date = dateo.getTime(); if (!children.get(1).evaluate(tuple, ptr)) { return false; } - DateTimeZone timezoneFrom = JodaTimezoneCache.getInstance(ptr); + TimeZone timezoneFrom = getTimezoneFromCache(Bytes.toString(ptr.get(), ptr.getOffset(), ptr.getLength())); if (!children.get(2).evaluate(tuple, ptr)) { return false; } - DateTimeZone timezoneTo = JodaTimezoneCache.getInstance(ptr); + TimeZone timezoneTo = TimeZone.getTimeZone(Bytes.toString(ptr.get(), ptr.getOffset(), ptr.getLength())); + + long dateInUtc = date - timezoneFrom.getOffset(date); + long dateInTo = dateInUtc + timezoneTo.getOffset(dateInUtc); + + ptr.set(PDate.INSTANCE.toBytes(new Date(dateInTo))); - long convertedDate = date - timezoneFrom.getOffset(date) + timezoneTo.getOffset(date); - byte[] outBytes = new byte[8]; - PDate.INSTANCE.getCodec().encodeLong(convertedDate, outBytes, 0); - ptr.set(outBytes); return true; } + private TimeZone getTimezoneFromCache(String timezone) throws IllegalDataException { + if (!cachedTimeZones.containsKey(timezone)) { + TimeZone tz = TimeZone.getTimeZone(timezone); + if (!tz.getID().equals(timezone)) { + throw new IllegalDataException("Invalid timezone " + timezone); + } + cachedTimeZones.put(timezone, tz); + return tz; + } + return cachedTimeZones.get(timezone); + } + @Override public PDataType getDataType() { return PDate.INSTANCE; http://git-wip-us.apache.org/repos/asf/phoenix/blob/66411e77/phoenix-core/src/main/java/org/apache/phoenix/expression/function/TimezoneOffsetFunction.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/TimezoneOffsetFunction.java b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/TimezoneOffsetFunction.java index 8c70346..2cfbc25 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/expression/function/TimezoneOffsetFunction.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/expression/function/TimezoneOffsetFunction.java @@ -18,18 +18,22 @@ package org.apache.phoenix.expression.function; +import java.sql.Date; import java.sql.SQLException; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.TimeZone; import org.apache.hadoop.hbase.io.ImmutableBytesWritable; -import org.apache.phoenix.cache.JodaTimezoneCache; +import org.apache.hadoop.hbase.util.Bytes; import org.apache.phoenix.expression.Expression; import org.apache.phoenix.parse.FunctionParseNode; +import org.apache.phoenix.schema.IllegalDataException; import org.apache.phoenix.schema.types.PDate; import org.apache.phoenix.schema.types.PInteger; import org.apache.phoenix.schema.types.PDataType; import org.apache.phoenix.schema.types.PVarchar; import org.apache.phoenix.schema.tuple.Tuple; -import org.joda.time.DateTimeZone; /** * Returns offset (shift in minutes) of timezone at particular datetime in minutes. @@ -41,6 +45,7 @@ public class TimezoneOffsetFunction extends ScalarFunction { public static final String NAME = "TIMEZONE_OFFSET"; private static final int MILLIS_TO_MINUTES = 60 * 1000; + private final Map<String, TimeZone> cachedTimeZones = new HashMap<String, TimeZone>(); public TimezoneOffsetFunction() { } @@ -59,14 +64,24 @@ public class TimezoneOffsetFunction extends ScalarFunction { if (!children.get(0).evaluate(tuple, ptr)) { return false; } - DateTimeZone timezoneInstance = JodaTimezoneCache.getInstance(ptr); + + String timezone = Bytes.toString(ptr.get(), ptr.getOffset(), ptr.getLength()); if (!children.get(1).evaluate(tuple, ptr)) { return false; } - long date = PDate.INSTANCE.getCodec().decodeLong(ptr, children.get(1).getSortOrder()); - int offset = timezoneInstance.getOffset(date); + if (!cachedTimeZones.containsKey(timezone)) { + TimeZone tz = TimeZone.getTimeZone(timezone); + if (!tz.getID().equals(timezone)) { + throw new IllegalDataException("Invalid timezone " + timezone); + } + cachedTimeZones.put(timezone, tz); + } + + Date date = (Date) PDate.INSTANCE.toObject(ptr, children.get(1).getSortOrder()); + int offset = cachedTimeZones.get(timezone).getOffset(date.getTime()); + ptr.set(PInteger.INSTANCE.toBytes(offset / MILLIS_TO_MINUTES)); return true; } http://git-wip-us.apache.org/repos/asf/phoenix/blob/66411e77/phoenix-core/src/test/java/org/apache/phoenix/cache/JodaTimezoneCacheTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/cache/JodaTimezoneCacheTest.java b/phoenix-core/src/test/java/org/apache/phoenix/cache/JodaTimezoneCacheTest.java deleted file mode 100644 index f388703..0000000 --- a/phoenix-core/src/test/java/org/apache/phoenix/cache/JodaTimezoneCacheTest.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Copyright 2015 Apache Software Foundation. - * - * Licensed 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.phoenix.cache; - -import java.nio.ByteBuffer; -import org.apache.hadoop.hbase.io.ImmutableBytesWritable; -import org.apache.hadoop.hbase.util.Bytes; -import org.apache.phoenix.schema.IllegalDataException; -import org.joda.time.DateTimeZone; -import static org.junit.Assert.assertTrue; -import org.junit.Test; - -public class JodaTimezoneCacheTest { - - @Test - public void testGetInstanceByteBufferUTC() { - DateTimeZone instance = JodaTimezoneCache.getInstance(ByteBuffer.wrap(Bytes.toBytes("UTC"))); - assertTrue(instance instanceof DateTimeZone); - } - - @Test - public void testGetInstanceString() { - DateTimeZone instance = JodaTimezoneCache.getInstance("America/St_Vincent"); - assertTrue(instance instanceof DateTimeZone); - } - - @Test(expected = IllegalDataException.class) - public void testGetInstanceStringUnknown() { - JodaTimezoneCache.getInstance("SOME_UNKNOWN_TIMEZONE"); - } - - @Test - public void testGetInstanceImmutableBytesWritable() { - ImmutableBytesWritable ptr = new ImmutableBytesWritable(Bytes.toBytes("Europe/Isle_of_Man")); - DateTimeZone instance = JodaTimezoneCache.getInstance(ptr); - assertTrue(instance instanceof DateTimeZone); - } -} http://git-wip-us.apache.org/repos/asf/phoenix/blob/66411e77/pom.xml ---------------------------------------------------------------------- diff --git a/pom.xml b/pom.xml index 0ccfd54..9194dc0 100644 --- a/pom.xml +++ b/pom.xml @@ -102,7 +102,7 @@ <commons-codec.version>1.7</commons-codec.version> <htrace.version>2.04</htrace.version> <collections.version>3.2.1</collections.version> - <jodatime.version>2.7</jodatime.version> + <jodatime.version>2.3</jodatime.version> <!-- Test Dependencies --> <mockito-all.version>1.8.5</mockito-all.version>
