You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@phoenix.apache.org by chrajeshbabu <gi...@git.apache.org> on 2015/04/25 01:08:27 UTC

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

GitHub user chrajeshbabu opened a pull request:

    https://github.com/apache/phoenix/pull/77

    PHOENIX-538 Support UDFs

    Patch to support UDFs. It mainly includes
    - create temporary/permanent function query parsing
    - storing function info
    - dynamically loading udf jars.
    - resolve functions 
    - making use of udfs in different queries
    - drop function.
    - it tests

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

    $ git pull https://github.com/chrajeshbabu/phoenix master

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

    https://github.com/apache/phoenix/pull/77.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 #77
    
----
commit a0c62d52492167b0d7c3d7b2036de8acfb762d92
Author: Rajeshbabu Chintaguntla <ra...@apache.org>
Date:   2015-04-24T23:03:55Z

    PHOENIX-538 Support UDFs

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by samarthjain <gi...@git.apache.org>.
Github user samarthjain commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098070
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java ---
    @@ -383,6 +448,85 @@ protected TableRef createTableRef(NamedTableNode tableNode, boolean updateCacheI
                 return tableRef;
             }
     
    +        @Override
    +        public List<PFunction> getFunctions() {
    +            return functions;
    +        }
    +
    +        protected List<PFunction> createFunctionRef(List<String> functionNames, boolean updateCacheImmediately) throws SQLException {
    --- End diff --
    
    Minor nit: is this function overriden somewhere? If not, it could be made private


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by samarthjain <gi...@git.apache.org>.
Github user samarthjain commented on the pull request:

    https://github.com/apache/phoenix/pull/77#issuecomment-96133889
  
    Almost there Rajesh. Just some minor nits and questions, otherwise LGTM.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by chrajeshbabu <gi...@git.apache.org>.
Github user chrajeshbabu commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098873
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/expression/function/UDFExpression.java ---
    @@ -0,0 +1,217 @@
    +/*
    + * 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.phoenix.expression.function;
    +
    +import static org.apache.phoenix.query.QueryServices.DYNAMIC_JARS_DIR_KEY;
    +
    +import java.io.DataInput;
    +import java.io.DataOutput;
    +import java.io.IOException;
    +import java.lang.reflect.Constructor;
    +import java.lang.reflect.InvocationTargetException;
    +import java.util.List;
    +import java.util.concurrent.ConcurrentMap;
    +import java.util.concurrent.locks.Lock;
    +
    +import org.apache.commons.logging.Log;
    +import org.apache.commons.logging.LogFactory;
    +import org.apache.hadoop.conf.Configuration;
    +import org.apache.hadoop.fs.Path;
    +import org.apache.hadoop.hbase.HBaseConfiguration;
    +import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
    +import org.apache.hadoop.hbase.util.DynamicClassLoader;
    +import org.apache.hadoop.hbase.util.KeyLocker;
    +import org.apache.hadoop.io.WritableUtils;
    +import org.apache.phoenix.compile.KeyPart;
    +import org.apache.phoenix.expression.Expression;
    +import org.apache.phoenix.expression.visitor.ExpressionVisitor;
    +import org.apache.phoenix.parse.PFunction;
    +import org.apache.phoenix.schema.PName;
    +import org.apache.phoenix.schema.PNameFactory;
    +import org.apache.phoenix.schema.tuple.Tuple;
    +import org.apache.phoenix.schema.types.PDataType;
    +
    +import com.google.common.annotations.VisibleForTesting;
    +import com.google.common.collect.MapMaker;
    +
    +public class UDFExpression extends ScalarFunction {
    --- End diff --
    
                        if (expression.getDeterminism() != Determinism.ALWAYS) {
                            throw new SQLExceptionInfo.Builder(SQLExceptionCode.NON_DETERMINISTIC_EXPRESSION_NOT_ALLOWED_IN_INDEX).build().buildException();
                        }
    
    Here it's the reason for failure James...getting problem with functional indexes only.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by chrajeshbabu <gi...@git.apache.org>.
Github user chrajeshbabu commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098659
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/cache/GlobalCache.java ---
    @@ -157,4 +159,18 @@ public TenantCache getChildTenantCache(ImmutableBytesWritable tenantId) {
             }
             return tenantCache;
         }
    +
    +    public static class FunctionBytesPtr extends ImmutableBytesPtr {
    +
    +        public FunctionBytesPtr(byte[] key) {
    +            super(key);
    +        }
    +
    +        @Override
    +        public boolean equals(Object obj) {
    --- End diff --
    
    I will override the hashcode and just call super.hashCode() then we won't get checkstyle or findbug warning.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by chrajeshbabu <gi...@git.apache.org>.
Github user chrajeshbabu closed the pull request at:

    https://github.com/apache/phoenix/pull/77


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by chrajeshbabu <gi...@git.apache.org>.
Github user chrajeshbabu commented on the pull request:

    https://github.com/apache/phoenix/pull/77#issuecomment-96969719
  
    It's committed. Hence closing.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by JamesRTaylor <gi...@git.apache.org>.
Github user JamesRTaylor commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29102300
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/expression/function/UDFExpression.java ---
    @@ -0,0 +1,217 @@
    +/*
    + * 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.phoenix.expression.function;
    +
    +import static org.apache.phoenix.query.QueryServices.DYNAMIC_JARS_DIR_KEY;
    +
    +import java.io.DataInput;
    +import java.io.DataOutput;
    +import java.io.IOException;
    +import java.lang.reflect.Constructor;
    +import java.lang.reflect.InvocationTargetException;
    +import java.util.List;
    +import java.util.concurrent.ConcurrentMap;
    +import java.util.concurrent.locks.Lock;
    +
    +import org.apache.commons.logging.Log;
    +import org.apache.commons.logging.LogFactory;
    +import org.apache.hadoop.conf.Configuration;
    +import org.apache.hadoop.fs.Path;
    +import org.apache.hadoop.hbase.HBaseConfiguration;
    +import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
    +import org.apache.hadoop.hbase.util.DynamicClassLoader;
    +import org.apache.hadoop.hbase.util.KeyLocker;
    +import org.apache.hadoop.io.WritableUtils;
    +import org.apache.phoenix.compile.KeyPart;
    +import org.apache.phoenix.expression.Expression;
    +import org.apache.phoenix.expression.visitor.ExpressionVisitor;
    +import org.apache.phoenix.parse.PFunction;
    +import org.apache.phoenix.schema.PName;
    +import org.apache.phoenix.schema.PNameFactory;
    +import org.apache.phoenix.schema.tuple.Tuple;
    +import org.apache.phoenix.schema.types.PDataType;
    +
    +import com.google.common.annotations.VisibleForTesting;
    +import com.google.common.collect.MapMaker;
    +
    +public class UDFExpression extends ScalarFunction {
    --- End diff --
    
    I see - forgot about that, and we shouldn't change that. Instead, in the RowProjector constructor we'll want to set cloneRequired to true if any of the expressions use a UDF. The easiest way is probably to add a boolean hasUDFs to the RowProjector, add a ColumnResolver.hasUDFs() that returns true if !functions.isEmpty(). This will still clone if a UDF is used in the WHERE clause, but not in the SELECT expressions, but that's ok (it won't harm anything).
        


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by JamesRTaylor <gi...@git.apache.org>.
Github user JamesRTaylor commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098825
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/expression/function/UDFExpression.java ---
    @@ -0,0 +1,217 @@
    +/*
    + * 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.phoenix.expression.function;
    +
    +import static org.apache.phoenix.query.QueryServices.DYNAMIC_JARS_DIR_KEY;
    +
    +import java.io.DataInput;
    +import java.io.DataOutput;
    +import java.io.IOException;
    +import java.lang.reflect.Constructor;
    +import java.lang.reflect.InvocationTargetException;
    +import java.util.List;
    +import java.util.concurrent.ConcurrentMap;
    +import java.util.concurrent.locks.Lock;
    +
    +import org.apache.commons.logging.Log;
    +import org.apache.commons.logging.LogFactory;
    +import org.apache.hadoop.conf.Configuration;
    +import org.apache.hadoop.fs.Path;
    +import org.apache.hadoop.hbase.HBaseConfiguration;
    +import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
    +import org.apache.hadoop.hbase.util.DynamicClassLoader;
    +import org.apache.hadoop.hbase.util.KeyLocker;
    +import org.apache.hadoop.io.WritableUtils;
    +import org.apache.phoenix.compile.KeyPart;
    +import org.apache.phoenix.expression.Expression;
    +import org.apache.phoenix.expression.visitor.ExpressionVisitor;
    +import org.apache.phoenix.parse.PFunction;
    +import org.apache.phoenix.schema.PName;
    +import org.apache.phoenix.schema.PNameFactory;
    +import org.apache.phoenix.schema.tuple.Tuple;
    +import org.apache.phoenix.schema.types.PDataType;
    +
    +import com.google.common.annotations.VisibleForTesting;
    +import com.google.common.collect.MapMaker;
    +
    +public class UDFExpression extends ScalarFunction {
    --- End diff --
    
    Having getDeterminism() return PER_INVOCATION is the only way to get each thread to have its own copy of the expression tree. Can  you file a JIRA with the failing test and exception? Is it only for functional indexing that you run into an issue?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by chrajeshbabu <gi...@git.apache.org>.
Github user chrajeshbabu commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29103995
  
    --- Diff: phoenix-core/src/main/antlr3/PhoenixSQL.g ---
    @@ -114,6 +114,15 @@ tokens
         ASYNC='async';
         SAMPLING='sampling';
         UNION='union';
    +    FUNCTION='function';
    +    AS='as';
    +    REPLACE='replace';
    --- End diff --
    
    Removed replace currently.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by chrajeshbabu <gi...@git.apache.org>.
Github user chrajeshbabu commented on the pull request:

    https://github.com/apache/phoenix/pull/77#issuecomment-96276745
  
    Thanks @JamesRTaylor  @samarthjain 
    I have addressed the review comments and added to pull request. If it's ok I will commit this tomorrow morning IST and work on subtasks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by samarthjain <gi...@git.apache.org>.
Github user samarthjain commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098063
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/compile/CreateFunctionCompiler.java ---
    @@ -0,0 +1,84 @@
    +/*
    + * 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.phoenix.compile;
    +
    +import java.sql.ParameterMetaData;
    +import java.sql.SQLException;
    +import java.sql.SQLFeatureNotSupportedException;
    +import java.util.Collections;
    +
    +import org.apache.hadoop.hbase.client.Scan;
    +import org.apache.phoenix.execute.MutationState;
    +import org.apache.phoenix.jdbc.PhoenixConnection;
    +import org.apache.phoenix.jdbc.PhoenixStatement;
    +import org.apache.phoenix.parse.CreateFunctionStatement;
    +import org.apache.phoenix.schema.MetaDataClient;
    +
    +public class CreateFunctionCompiler {
    +
    +    private final PhoenixStatement statement;
    +    
    +    public CreateFunctionCompiler(PhoenixStatement statement) {
    +        this.statement = statement;
    +    }
    +
    +    public MutationPlan compile(final CreateFunctionStatement create) throws SQLException {
    +        if(create.isReplace()) {
    +            throw new SQLFeatureNotSupportedException();
    +        }
    +        final PhoenixConnection connection = statement.getConnection();
    +        PhoenixConnection connectionToBe = connection;
    +        Scan scan = new Scan();
    +        final StatementContext context = new StatementContext(statement, FromCompiler.EMPTY_TABLE_RESOLVER, scan, new SequenceManager(statement));
    --- End diff --
    
    Minor nit: You can use the other constructor here public StatementContext(PhoenixStatement statement)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by chrajeshbabu <gi...@git.apache.org>.
Github user chrajeshbabu commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098680
  
    --- Diff: phoenix-core/src/test/java/org/apache/phoenix/parse/QueryParserTest.java ---
    @@ -289,24 +289,6 @@ public void testNegativeCountStar() throws Exception {
         }
     
         @Test
    -    public void testUnknownFunction() throws Exception {
    --- End diff --
    
    Now the exception will be thrown during compilation not while parsing because while parsing not able check whether udf enabled or not. That's why removed it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by chrajeshbabu <gi...@git.apache.org>.
Github user chrajeshbabu commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098632
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/compile/FromCompiler.java ---
    @@ -383,6 +448,85 @@ protected TableRef createTableRef(NamedTableNode tableNode, boolean updateCacheI
                 return tableRef;
             }
     
    +        @Override
    +        public List<PFunction> getFunctions() {
    +            return functions;
    +        }
    +
    +        protected List<PFunction> createFunctionRef(List<String> functionNames, boolean updateCacheImmediately) throws SQLException {
    --- End diff --
    
    no will change it to private. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by samarthjain <gi...@git.apache.org>.
Github user samarthjain commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098156
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/expression/function/UDFExpression.java ---
    @@ -0,0 +1,217 @@
    +/*
    + * 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.phoenix.expression.function;
    +
    +import static org.apache.phoenix.query.QueryServices.DYNAMIC_JARS_DIR_KEY;
    +
    +import java.io.DataInput;
    +import java.io.DataOutput;
    +import java.io.IOException;
    +import java.lang.reflect.Constructor;
    +import java.lang.reflect.InvocationTargetException;
    +import java.util.List;
    +import java.util.concurrent.ConcurrentMap;
    +import java.util.concurrent.locks.Lock;
    +
    +import org.apache.commons.logging.Log;
    +import org.apache.commons.logging.LogFactory;
    +import org.apache.hadoop.conf.Configuration;
    +import org.apache.hadoop.fs.Path;
    +import org.apache.hadoop.hbase.HBaseConfiguration;
    +import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
    +import org.apache.hadoop.hbase.util.DynamicClassLoader;
    +import org.apache.hadoop.hbase.util.KeyLocker;
    +import org.apache.hadoop.io.WritableUtils;
    +import org.apache.phoenix.compile.KeyPart;
    +import org.apache.phoenix.expression.Expression;
    +import org.apache.phoenix.expression.visitor.ExpressionVisitor;
    +import org.apache.phoenix.parse.PFunction;
    +import org.apache.phoenix.schema.PName;
    +import org.apache.phoenix.schema.PNameFactory;
    +import org.apache.phoenix.schema.tuple.Tuple;
    +import org.apache.phoenix.schema.types.PDataType;
    +
    +import com.google.common.annotations.VisibleForTesting;
    +import com.google.common.collect.MapMaker;
    +
    +public class UDFExpression extends ScalarFunction {
    +    
    +    private static Configuration config = HBaseConfiguration.create();
    +
    +    private static final ConcurrentMap<PName, DynamicClassLoader> tenantIdSpecificCls =
    +            new MapMaker().concurrencyLevel(3).weakValues().makeMap();
    +
    +    private static final ConcurrentMap<String, DynamicClassLoader> pathSpecificCls =
    +            new MapMaker().concurrencyLevel(3).weakValues().makeMap();
    +
    +    public static final Log LOG = LogFactory.getLog(UDFExpression.class);
    +    
    +    /**
    +     * A locker used to synchronize class loader initialization per tenant id.
    +     */
    +    private static final KeyLocker<String> locker = new KeyLocker<String>();
    +
    +    /**
    +     * A locker used to synchronize class loader initialization per jar path.
    +     */
    +    private static final KeyLocker<String> pathLocker = new KeyLocker<String>();
    +
    +    private PName tenantId;
    +    private String functionClassName;
    +    private String jarPath;
    +    private ScalarFunction udfFunction;
    +    
    +    public UDFExpression() {
    +    }
    +
    +    public UDFExpression(List<Expression> children,PFunction functionInfo) {
    +        super(children);
    +        this.tenantId =
    +                functionInfo.getTenantId() == null ? PName.EMPTY_NAME : functionInfo.getTenantId();
    +        this.functionClassName = functionInfo.getClassName();
    +        this.jarPath = functionInfo.getJarPath();
    +        constructUDFFunction();
    +    }
    +
    +    @Override
    +    public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
    +        return udfFunction.evaluate(tuple, ptr);
    +    }
    +
    +    @Override
    +    public <T> T accept(ExpressionVisitor<T> visitor) {
    +        return udfFunction.accept(visitor);
    +    }
    +
    +    @Override
    +    public PDataType getDataType() {
    +        return udfFunction.getDataType();
    +    }
    +
    +    @Override
    +    public String getName() {
    +        return udfFunction.getName();
    +    }
    +
    +    @Override
    +    public OrderPreserving preservesOrder() {
    +        return udfFunction.preservesOrder();
    +    }
    +
    +    @Override
    +    public KeyPart newKeyPart(KeyPart childPart) {
    +        return udfFunction.newKeyPart(childPart);
    +    }
    +
    +    @Override
    +    public int getKeyFormationTraversalIndex() {
    +        return udfFunction.getKeyFormationTraversalIndex();
    +    }
    +
    +    @Override
    +    public void write(DataOutput output) throws IOException {
    +        super.write(output);
    +        WritableUtils.writeString(output, tenantId.getString());
    +        WritableUtils.writeString(output, this.functionClassName);
    +        if(this.jarPath == null) {
    +            WritableUtils.writeString(output, "");
    +        } else {
    +            WritableUtils.writeString(output, this.jarPath);
    +        }
    +    }
    +    
    +    @Override
    +    public void readFields(DataInput input) throws IOException {
    +        super.readFields(input);
    +        this.tenantId = PNameFactory.newName(WritableUtils.readString(input));
    +        this.functionClassName = WritableUtils.readString(input);
    +        String str = WritableUtils.readString(input);
    +        this.jarPath = str.length() == 0 ? null: str;
    +        constructUDFFunction();
    +    }
    +
    +    private void constructUDFFunction() {
    +        try {
    +            DynamicClassLoader classLoader = getClassLoader(this.tenantId, this.jarPath);
    +            Class<?> clazz = classLoader.loadClass(this.functionClassName);
    +            Constructor<?> constructor = clazz.getConstructor(List.class);
    +            udfFunction = (ScalarFunction)constructor.newInstance(this.children);
    +        } catch (ClassNotFoundException | NoSuchMethodException | SecurityException
    +                | InstantiationException | IllegalAccessException | IllegalArgumentException
    +                | InvocationTargetException e) {
    +            throw new RuntimeException(e);
    +        }
    +    }
    +
    +    public static DynamicClassLoader getClassLoader(final PName tenantId, final String jarPath) {
    +        DynamicClassLoader cl = tenantIdSpecificCls.get(tenantId);
    +        String parent = null;
    +        if (cl != null) return cl;
    +        if(jarPath != null && !jarPath.isEmpty()) {
    +            cl = pathSpecificCls.get(jarPath);
    +            if (cl != null) return cl;
    +            Path path = new Path(jarPath);
    +            if(jarPath.endsWith(".jar")) {
    +                parent = path.getParent().toString();
    +            } else {
    +                parent = path.toString();
    +            }
    +        }
    +        if (jarPath == null || jarPath.isEmpty() || config.get(DYNAMIC_JARS_DIR_KEY) != null
    +                && (parent != null && parent.equals(config.get(DYNAMIC_JARS_DIR_KEY)))) {
    +            Lock lock = locker.acquireLock(tenantId.getString());
    --- End diff --
    
    Rajesh says: Just want to clarify one thing here the map is concurrent map so get and putifAbsent will be atomic but there is a chance like between get and put if other client already puts a class loader for the tenant then the classloader and configuration object created become garbage. That's why using locker. If that's ok I can do as you suggested. What do u say?
    
    Samarth says: How expensive is creating a new classloader and config? To me it doesn't look too expensive. So I wouldn't worry about creating and discarding the two. Besides that race condition of another thread winning the race between get() and putIfAbsent is not very likely. So to sum it up, yes, I would get rid of the KeyLocker altogether.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by chrajeshbabu <gi...@git.apache.org>.
Github user chrajeshbabu commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098664
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/compile/CreateFunctionCompiler.java ---
    @@ -0,0 +1,84 @@
    +/*
    + * 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.phoenix.compile;
    +
    +import java.sql.ParameterMetaData;
    +import java.sql.SQLException;
    +import java.sql.SQLFeatureNotSupportedException;
    +import java.util.Collections;
    +
    +import org.apache.hadoop.hbase.client.Scan;
    +import org.apache.phoenix.execute.MutationState;
    +import org.apache.phoenix.jdbc.PhoenixConnection;
    +import org.apache.phoenix.jdbc.PhoenixStatement;
    +import org.apache.phoenix.parse.CreateFunctionStatement;
    +import org.apache.phoenix.schema.MetaDataClient;
    +
    +public class CreateFunctionCompiler {
    +
    +    private final PhoenixStatement statement;
    +    
    +    public CreateFunctionCompiler(PhoenixStatement statement) {
    +        this.statement = statement;
    +    }
    +
    +    public MutationPlan compile(final CreateFunctionStatement create) throws SQLException {
    +        if(create.isReplace()) {
    +            throw new SQLFeatureNotSupportedException();
    +        }
    +        final PhoenixConnection connection = statement.getConnection();
    +        PhoenixConnection connectionToBe = connection;
    +        Scan scan = new Scan();
    +        final StatementContext context = new StatementContext(statement, FromCompiler.EMPTY_TABLE_RESOLVER, scan, new SequenceManager(statement));
    --- End diff --
    
    That would be great. Will change.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by chrajeshbabu <gi...@git.apache.org>.
Github user chrajeshbabu commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098669
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java ---
    @@ -700,6 +1007,29 @@ private PTable loadTable(RegionCoprocessorEnvironment env, byte[] key,
             return null;
         }
     
    +    private PFunction loadFunction(RegionCoprocessorEnvironment env, byte[] key,
    +            ImmutableBytesPtr cacheKey, long clientTimeStamp, long asOfTimeStamp)
    +            throws IOException, SQLException {
    +            HRegion region = env.getRegion();
    +            Cache<ImmutableBytesPtr,PMetaDataEntity> metaDataCache = GlobalCache.getInstance(this.env).getMetaDataCache();
    +            PFunction function = (PFunction)metaDataCache.getIfPresent(cacheKey);
    +            // We always cache the latest version - fault in if not in cache
    +            if (function != null) {
    +                return function;
    +            }
    +            ArrayList<byte[]> arrayList = new ArrayList<byte[]>(1);
    +            arrayList.add(key);
    +            List<PFunction> functions = buildFunctions(arrayList, region, asOfTimeStamp);
    +            if(functions != null) return functions.get(0);
    +            // if not found then check if newer table already exists and add delete marker for timestamp
    --- End diff --
    
    yes


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by chrajeshbabu <gi...@git.apache.org>.
Github user chrajeshbabu commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098894
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/expression/function/UDFExpression.java ---
    @@ -0,0 +1,217 @@
    +/*
    + * 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.phoenix.expression.function;
    +
    +import static org.apache.phoenix.query.QueryServices.DYNAMIC_JARS_DIR_KEY;
    +
    +import java.io.DataInput;
    +import java.io.DataOutput;
    +import java.io.IOException;
    +import java.lang.reflect.Constructor;
    +import java.lang.reflect.InvocationTargetException;
    +import java.util.List;
    +import java.util.concurrent.ConcurrentMap;
    +import java.util.concurrent.locks.Lock;
    +
    +import org.apache.commons.logging.Log;
    +import org.apache.commons.logging.LogFactory;
    +import org.apache.hadoop.conf.Configuration;
    +import org.apache.hadoop.fs.Path;
    +import org.apache.hadoop.hbase.HBaseConfiguration;
    +import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
    +import org.apache.hadoop.hbase.util.DynamicClassLoader;
    +import org.apache.hadoop.hbase.util.KeyLocker;
    +import org.apache.hadoop.io.WritableUtils;
    +import org.apache.phoenix.compile.KeyPart;
    +import org.apache.phoenix.expression.Expression;
    +import org.apache.phoenix.expression.visitor.ExpressionVisitor;
    +import org.apache.phoenix.parse.PFunction;
    +import org.apache.phoenix.schema.PName;
    +import org.apache.phoenix.schema.PNameFactory;
    +import org.apache.phoenix.schema.tuple.Tuple;
    +import org.apache.phoenix.schema.types.PDataType;
    +
    +import com.google.common.annotations.VisibleForTesting;
    +import com.google.common.collect.MapMaker;
    +
    +public class UDFExpression extends ScalarFunction {
    --- End diff --
    
    java.sql.SQLException: ERROR 521 (42898): Non-deterministic expression not allowed in an index
    	at org.apache.phoenix.exception.SQLExceptionCode$Factory$1.newException(SQLExceptionCode.java:386)
    	at org.apache.phoenix.exception.SQLExceptionInfo.buildException(SQLExceptionInfo.java:145)
    	at org.apache.phoenix.schema.MetaDataClient.createIndex(MetaDataClient.java:1179)
    	at org.apache.phoenix.compile.CreateIndexCompiler$1.execute(CreateIndexCompiler.java:95)
    	at org.apache.phoenix.jdbc.PhoenixStatement$3.call(PhoenixStatement.java:303)
    	at org.apache.phoenix.jdbc.PhoenixStatement$3.call(PhoenixStatement.java:1)
    	at org.apache.phoenix.call.CallRunner.run(CallRunner.java:53)
    	at org.apache.phoenix.jdbc.PhoenixStatement.executeMutation(PhoenixStatement.java:294)
    	at org.apache.phoenix.jdbc.PhoenixStatement.execute(PhoenixStatement.java:1247)
    	at org.apache.phoenix.end2end.UserDefinedFunctionsIT.testFunctionalIndexesWithUDFFunction(UserDefinedFunctionsIT.java:509)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by chrajeshbabu <gi...@git.apache.org>.
Github user chrajeshbabu commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29103983
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/expression/function/UDFExpression.java ---
    @@ -0,0 +1,217 @@
    +/*
    + * 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.phoenix.expression.function;
    +
    +import static org.apache.phoenix.query.QueryServices.DYNAMIC_JARS_DIR_KEY;
    +
    +import java.io.DataInput;
    +import java.io.DataOutput;
    +import java.io.IOException;
    +import java.lang.reflect.Constructor;
    +import java.lang.reflect.InvocationTargetException;
    +import java.util.List;
    +import java.util.concurrent.ConcurrentMap;
    +import java.util.concurrent.locks.Lock;
    +
    +import org.apache.commons.logging.Log;
    +import org.apache.commons.logging.LogFactory;
    +import org.apache.hadoop.conf.Configuration;
    +import org.apache.hadoop.fs.Path;
    +import org.apache.hadoop.hbase.HBaseConfiguration;
    +import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
    +import org.apache.hadoop.hbase.util.DynamicClassLoader;
    +import org.apache.hadoop.hbase.util.KeyLocker;
    +import org.apache.hadoop.io.WritableUtils;
    +import org.apache.phoenix.compile.KeyPart;
    +import org.apache.phoenix.expression.Expression;
    +import org.apache.phoenix.expression.visitor.ExpressionVisitor;
    +import org.apache.phoenix.parse.PFunction;
    +import org.apache.phoenix.schema.PName;
    +import org.apache.phoenix.schema.PNameFactory;
    +import org.apache.phoenix.schema.tuple.Tuple;
    +import org.apache.phoenix.schema.types.PDataType;
    +
    +import com.google.common.annotations.VisibleForTesting;
    +import com.google.common.collect.MapMaker;
    +
    +public class UDFExpression extends ScalarFunction {
    --- End diff --
    
    Added this James. Thanks for alternative soln.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by JamesRTaylor <gi...@git.apache.org>.
Github user JamesRTaylor commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29102639
  
    --- Diff: phoenix-core/src/main/antlr3/PhoenixSQL.g ---
    @@ -114,6 +114,15 @@ tokens
         ASYNC='async';
         SAMPLING='sampling';
         UNION='union';
    +    FUNCTION='function';
    +    AS='as';
    +    REPLACE='replace';
    --- End diff --
    
    If support for REPLACE won't make this pull request (which is fine b/c it's already big enough), we should just remove it from the grammar for now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by samarthjain <gi...@git.apache.org>.
Github user samarthjain commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098050
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/cache/GlobalCache.java ---
    @@ -157,4 +159,18 @@ public TenantCache getChildTenantCache(ImmutableBytesWritable tenantId) {
             }
             return tenantCache;
         }
    +
    +    public static class FunctionBytesPtr extends ImmutableBytesPtr {
    +
    +        public FunctionBytesPtr(byte[] key) {
    +            super(key);
    +        }
    +
    +        @Override
    +        public boolean equals(Object obj) {
    --- End diff --
    
    Should you be overriding the hashCode() method too?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by chrajeshbabu <gi...@git.apache.org>.
Github user chrajeshbabu commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098678
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/expression/function/UDFExpression.java ---
    @@ -0,0 +1,217 @@
    +/*
    + * 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.phoenix.expression.function;
    +
    +import static org.apache.phoenix.query.QueryServices.DYNAMIC_JARS_DIR_KEY;
    +
    +import java.io.DataInput;
    +import java.io.DataOutput;
    +import java.io.IOException;
    +import java.lang.reflect.Constructor;
    +import java.lang.reflect.InvocationTargetException;
    +import java.util.List;
    +import java.util.concurrent.ConcurrentMap;
    +import java.util.concurrent.locks.Lock;
    +
    +import org.apache.commons.logging.Log;
    +import org.apache.commons.logging.LogFactory;
    +import org.apache.hadoop.conf.Configuration;
    +import org.apache.hadoop.fs.Path;
    +import org.apache.hadoop.hbase.HBaseConfiguration;
    +import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
    +import org.apache.hadoop.hbase.util.DynamicClassLoader;
    +import org.apache.hadoop.hbase.util.KeyLocker;
    +import org.apache.hadoop.io.WritableUtils;
    +import org.apache.phoenix.compile.KeyPart;
    +import org.apache.phoenix.expression.Expression;
    +import org.apache.phoenix.expression.visitor.ExpressionVisitor;
    +import org.apache.phoenix.parse.PFunction;
    +import org.apache.phoenix.schema.PName;
    +import org.apache.phoenix.schema.PNameFactory;
    +import org.apache.phoenix.schema.tuple.Tuple;
    +import org.apache.phoenix.schema.types.PDataType;
    +
    +import com.google.common.annotations.VisibleForTesting;
    +import com.google.common.collect.MapMaker;
    +
    +public class UDFExpression extends ScalarFunction {
    +    
    +    private static Configuration config = HBaseConfiguration.create();
    +
    +    private static final ConcurrentMap<PName, DynamicClassLoader> tenantIdSpecificCls =
    +            new MapMaker().concurrencyLevel(3).weakValues().makeMap();
    +
    +    private static final ConcurrentMap<String, DynamicClassLoader> pathSpecificCls =
    +            new MapMaker().concurrencyLevel(3).weakValues().makeMap();
    +
    +    public static final Log LOG = LogFactory.getLog(UDFExpression.class);
    +    
    +    /**
    +     * A locker used to synchronize class loader initialization per tenant id.
    +     */
    +    private static final KeyLocker<String> locker = new KeyLocker<String>();
    +
    +    /**
    +     * A locker used to synchronize class loader initialization per jar path.
    +     */
    +    private static final KeyLocker<String> pathLocker = new KeyLocker<String>();
    +
    +    private PName tenantId;
    +    private String functionClassName;
    +    private String jarPath;
    +    private ScalarFunction udfFunction;
    +    
    +    public UDFExpression() {
    +    }
    +
    +    public UDFExpression(List<Expression> children,PFunction functionInfo) {
    +        super(children);
    +        this.tenantId =
    +                functionInfo.getTenantId() == null ? PName.EMPTY_NAME : functionInfo.getTenantId();
    +        this.functionClassName = functionInfo.getClassName();
    +        this.jarPath = functionInfo.getJarPath();
    +        constructUDFFunction();
    +    }
    +
    +    @Override
    +    public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
    +        return udfFunction.evaluate(tuple, ptr);
    +    }
    +
    +    @Override
    +    public <T> T accept(ExpressionVisitor<T> visitor) {
    +        return udfFunction.accept(visitor);
    +    }
    +
    +    @Override
    +    public PDataType getDataType() {
    +        return udfFunction.getDataType();
    +    }
    +
    +    @Override
    +    public String getName() {
    +        return udfFunction.getName();
    +    }
    +
    +    @Override
    +    public OrderPreserving preservesOrder() {
    +        return udfFunction.preservesOrder();
    +    }
    +
    +    @Override
    +    public KeyPart newKeyPart(KeyPart childPart) {
    +        return udfFunction.newKeyPart(childPart);
    +    }
    +
    +    @Override
    +    public int getKeyFormationTraversalIndex() {
    +        return udfFunction.getKeyFormationTraversalIndex();
    +    }
    +
    +    @Override
    +    public void write(DataOutput output) throws IOException {
    +        super.write(output);
    +        WritableUtils.writeString(output, tenantId.getString());
    +        WritableUtils.writeString(output, this.functionClassName);
    +        if(this.jarPath == null) {
    +            WritableUtils.writeString(output, "");
    +        } else {
    +            WritableUtils.writeString(output, this.jarPath);
    +        }
    +    }
    +    
    +    @Override
    +    public void readFields(DataInput input) throws IOException {
    +        super.readFields(input);
    +        this.tenantId = PNameFactory.newName(WritableUtils.readString(input));
    +        this.functionClassName = WritableUtils.readString(input);
    +        String str = WritableUtils.readString(input);
    +        this.jarPath = str.length() == 0 ? null: str;
    +        constructUDFFunction();
    +    }
    +
    +    private void constructUDFFunction() {
    +        try {
    +            DynamicClassLoader classLoader = getClassLoader(this.tenantId, this.jarPath);
    +            Class<?> clazz = classLoader.loadClass(this.functionClassName);
    +            Constructor<?> constructor = clazz.getConstructor(List.class);
    +            udfFunction = (ScalarFunction)constructor.newInstance(this.children);
    +        } catch (ClassNotFoundException | NoSuchMethodException | SecurityException
    +                | InstantiationException | IllegalAccessException | IllegalArgumentException
    +                | InvocationTargetException e) {
    +            throw new RuntimeException(e);
    +        }
    +    }
    +
    +    public static DynamicClassLoader getClassLoader(final PName tenantId, final String jarPath) {
    +        DynamicClassLoader cl = tenantIdSpecificCls.get(tenantId);
    +        String parent = null;
    +        if (cl != null) return cl;
    +        if(jarPath != null && !jarPath.isEmpty()) {
    +            cl = pathSpecificCls.get(jarPath);
    +            if (cl != null) return cl;
    +            Path path = new Path(jarPath);
    +            if(jarPath.endsWith(".jar")) {
    +                parent = path.getParent().toString();
    +            } else {
    +                parent = path.toString();
    +            }
    +        }
    +        if (jarPath == null || jarPath.isEmpty() || config.get(DYNAMIC_JARS_DIR_KEY) != null
    +                && (parent != null && parent.equals(config.get(DYNAMIC_JARS_DIR_KEY)))) {
    +            Lock lock = locker.acquireLock(tenantId.getString());
    --- End diff --
    
    Sure then will remove it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by chrajeshbabu <gi...@git.apache.org>.
Github user chrajeshbabu commented on the pull request:

    https://github.com/apache/phoenix/pull/77#issuecomment-96145601
  
    Thanks Samarth for reviews. Will update patch addressing the comments.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by samarthjain <gi...@git.apache.org>.
Github user samarthjain commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098203
  
    --- Diff: phoenix-core/src/test/java/org/apache/phoenix/parse/QueryParserTest.java ---
    @@ -289,24 +289,6 @@ public void testNegativeCountStar() throws Exception {
         }
     
         @Test
    -    public void testUnknownFunction() throws Exception {
    --- End diff --
    
    Is there a reason why this test was removed? Are we no longer throwing an error? Or has the exception code changed? I would think if the user doesn't have load udf enabled then this error is still valid.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] phoenix pull request: PHOENIX-538 Support UDFs

Posted by samarthjain <gi...@git.apache.org>.
Github user samarthjain commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/77#discussion_r29098096
  
    --- Diff: phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java ---
    @@ -700,6 +1007,29 @@ private PTable loadTable(RegionCoprocessorEnvironment env, byte[] key,
             return null;
         }
     
    +    private PFunction loadFunction(RegionCoprocessorEnvironment env, byte[] key,
    +            ImmutableBytesPtr cacheKey, long clientTimeStamp, long asOfTimeStamp)
    +            throws IOException, SQLException {
    +            HRegion region = env.getRegion();
    +            Cache<ImmutableBytesPtr,PMetaDataEntity> metaDataCache = GlobalCache.getInstance(this.env).getMetaDataCache();
    +            PFunction function = (PFunction)metaDataCache.getIfPresent(cacheKey);
    +            // We always cache the latest version - fault in if not in cache
    +            if (function != null) {
    +                return function;
    +            }
    +            ArrayList<byte[]> arrayList = new ArrayList<byte[]>(1);
    +            arrayList.add(key);
    +            List<PFunction> functions = buildFunctions(arrayList, region, asOfTimeStamp);
    +            if(functions != null) return functions.get(0);
    +            // if not found then check if newer table already exists and add delete marker for timestamp
    --- End diff --
    
    newer function?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---