phoenix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sama...@apache.org
Subject phoenix git commit: Revert "PHOENIX-1722 Speedup CONVERT_TZ function (Vaclav Loffelmann)"
Date Sat, 28 Mar 2015 00:35:31 GMT
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 <samarth.jain@salesforce.com>
Authored: Fri Mar 27 17:34:38 2015 -0700
Committer: Samarth <samarth.jain@salesforce.com>
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>


Mime
View raw message