You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2021/10/13 12:19:21 UTC
[GitHub] [drill] luocooong opened a new pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
luocooong opened a new pull request #2332:
URL: https://github.com/apache/drill/pull/2332
# [DRILL-7863](https://issues.apache.org/jira/browse/DRILL-7863): Add Storage Plugin for Apache Phoenix
## Description
To-do lists :
- [x] core module
- [ ] testcontainers based on self-developed
- [ ] bugfix and integration test in real world
- [ ] code cleanup and javadocs
## Documentation
WIP
## Testing
WIP
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on a change in pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2332:
URL: https://github.com/apache/drill/pull/2332#discussion_r758421339
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixStoragePluginConfig.java
##########
@@ -0,0 +1,141 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.drill.common.PlanStringBuilder;
+import org.apache.drill.common.logical.AbstractSecuredStoragePluginConfig;
+import org.apache.drill.common.logical.security.CredentialsProvider;
+import org.apache.drill.exec.store.security.CredentialProviderUtils;
+import org.apache.drill.exec.store.security.UsernamePasswordCredentials;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+
+@JsonTypeName(PhoenixStoragePluginConfig.NAME)
+public class PhoenixStoragePluginConfig extends AbstractSecuredStoragePluginConfig {
+
+ public static final String NAME = "phoenix";
+ public static final String THIN_DRIVER_CLASS = "org.apache.phoenix.queryserver.client.Driver";
+ public static final String FAT_DRIVER_CLASS = "org.apache.phoenix.jdbc.PhoenixDriver";
+
+ private final String host;
+ private final int port;
+ private final String jdbcURL; // (options) Equal to host + port
+ private final Map<String, Object> props; // (options) See also http://phoenix.apache.org/tuning.html
+
+ @JsonCreator
+ public PhoenixStoragePluginConfig(
+ @JsonProperty("host") String host,
+ @JsonProperty("port") int port,
+ @JsonProperty("username") String username,
+ @JsonProperty("password") String password,
+ @JsonProperty("jdbcURL") String jdbcURL,
+ @JsonProperty("credentialsProvider") CredentialsProvider credentialsProvider,
+ @JsonProperty("props") Map<String, Object> props) {
+ super(CredentialProviderUtils.getCredentialsProvider(username, password, credentialsProvider), credentialsProvider == null);
+ this.host = host;
+ this.port = port == 0 ? 8765 : port;
+ this.jdbcURL = jdbcURL;
+ this.props = props == null ? Collections.emptyMap() : props;
+ }
+
+ @JsonIgnore
+ public UsernamePasswordCredentials getUsernamePasswordCredentials() {
+ return new UsernamePasswordCredentials(credentialsProvider);
+ }
+
+ @JsonProperty("host")
+ public String getHost() {
+ return host;
+ }
+
+ @JsonProperty("port")
+ public int getPort() {
+ return port;
+ }
+
+ @JsonProperty("username")
+ public String getUsername() {
+ if (directCredentials) {
+ return getUsernamePasswordCredentials().getUsername();
+ }
+ return null;
+ }
+
+ @JsonIgnore
+ @JsonProperty("password")
+ public String getPassword() {
+ if (directCredentials) {
+ return getUsernamePasswordCredentials().getPassword();
+ }
+ return null;
+ }
+
+ @JsonProperty("jdbcURL")
+ public String getJdbcURL() {
+ return jdbcURL;
+ }
+
+ @JsonProperty("props")
+ public Map<String, Object> getProps() {
+ return props;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (o == this) {
+ return true;
+ }
+ if (o == null || !(o instanceof PhoenixStoragePluginConfig)) {
+ return false;
+ }
+ PhoenixStoragePluginConfig config = (PhoenixStoragePluginConfig) o;
+ // URL first
+ if (StringUtils.isNotBlank(config.getJdbcURL())) {
+ return Objects.equals(this.jdbcURL, config.getJdbcURL());
+ }
+ // Then the host and port
+ return Objects.equals(this.host, config.getHost()) && Objects.equals(this.port, config.getPort());
+ }
+
+ @Override
+ public int hashCode() {
+ if (StringUtils.isNotBlank(jdbcURL)) {
+ return Objects.hash(jdbcURL);
+ }
+ return Objects.hash(host, port);
+ }
+
+ @Override
+ public String toString() {
+ return new PlanStringBuilder(PhoenixStoragePluginConfig.NAME)
+ .field("driverName", THIN_DRIVER_CLASS)
+ .field("host", host)
+ .field("port", port)
+ .field("jdbcURL", jdbcURL)
+ .field("props", props)
Review comment:
Just as an FYSA, if you're using the `PlanStringBuilder`, there is the `maskedField` function which will do that for you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] paul-rogers commented on a change in pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
paul-rogers commented on a change in pull request #2332:
URL: https://github.com/apache/drill/pull/2332#discussion_r758097799
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixReader.java
##########
@@ -0,0 +1,463 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.math.BigDecimal;
+import java.sql.Array;
+import java.sql.Date;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Time;
+import java.sql.Timestamp;
+import java.sql.Types;
+import java.util.Arrays;
+import java.util.Map;
+
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.physical.resultSet.ResultSetLoader;
+import org.apache.drill.exec.physical.resultSet.RowSetLoader;
+import org.apache.drill.exec.record.metadata.SchemaBuilder;
+import org.apache.drill.exec.vector.accessor.ColumnWriter;
+import org.apache.drill.exec.vector.accessor.ScalarWriter;
+import org.apache.drill.shaded.guava.com.google.common.collect.Maps;
+
+public class PhoenixReader {
+
+ private final RowSetLoader writer;
+ private final ColumnDefn[] columns;
+ private final ResultSet results;
+ private long count;
+
+ public PhoenixReader(ResultSetLoader loader, ColumnDefn[] columns, ResultSet results) {
+ this.writer = loader.writer();
+ this.columns = columns;
+ this.results = results;
+ }
+
+ public RowSetLoader getStorage() {
+ return writer;
+ }
+
+ public long getCount() {
+ return count;
+ }
+
+ /**
+ * Fetch and process one row.
+ * @return return true if one row is processed, return false if there is no next row.
+ * @throws SQLException
+ */
+ public boolean processRow() throws SQLException {
+ if (results.next()) {
+ writer.start();
+ for (int index = 0; index < columns.length; index++) {
+ if (columns[index].getSqlType() == Types.ARRAY) {
+ Array result = results.getArray(index + 1);
+ if (result != null) {
+ columns[index].load(result.getArray());
+ }
+ } else {
+ Object result = results.getObject(index + 1);
+ if (result != null) {
+ columns[index].load(result);
+ }
+ }
+ }
+ count++;
+ writer.save();
+ return true;
+ }
+ return false;
+ }
+
+ protected static final Map<Integer, MinorType> COLUMN_TYPE_MAP = Maps.newHashMap();
+
+ static {
+ // text
+ COLUMN_TYPE_MAP.put(Types.VARCHAR, MinorType.VARCHAR);
+ COLUMN_TYPE_MAP.put(Types.CHAR, MinorType.VARCHAR);
+ // numbers
+ COLUMN_TYPE_MAP.put(Types.BIGINT, MinorType.BIGINT);
+ COLUMN_TYPE_MAP.put(Types.INTEGER, MinorType.INT);
+ COLUMN_TYPE_MAP.put(Types.SMALLINT, MinorType.INT);
+ COLUMN_TYPE_MAP.put(Types.TINYINT, MinorType.INT);
+ COLUMN_TYPE_MAP.put(Types.DOUBLE, MinorType.FLOAT8);
+ COLUMN_TYPE_MAP.put(Types.FLOAT, MinorType.FLOAT4);
+ COLUMN_TYPE_MAP.put(Types.DECIMAL, MinorType.VARDECIMAL);
+ // time
+ COLUMN_TYPE_MAP.put(Types.DATE, MinorType.DATE);
+ COLUMN_TYPE_MAP.put(Types.TIME, MinorType.TIME);
+ COLUMN_TYPE_MAP.put(Types.TIMESTAMP, MinorType.TIMESTAMP);
+ // binary
+ COLUMN_TYPE_MAP.put(Types.BINARY, MinorType.VARBINARY); // Raw fixed length byte array. Mapped to byte[].
+ COLUMN_TYPE_MAP.put(Types.VARBINARY, MinorType.VARBINARY); // Raw variable length byte array.
+ // boolean
+ COLUMN_TYPE_MAP.put(Types.BOOLEAN, MinorType.BIT);
+ }
+
+ protected abstract static class ColumnDefn {
+
+ final String name;
+ final int index;
+ final int sqlType;
+ ColumnWriter writer;
+
+ public String getName() {
+ return name;
+ }
+
+ public int getIndex() {
+ return index;
+ }
+
+ public int getSqlType() {
+ return sqlType;
+ }
+
+ public ColumnDefn(String name, int index, int sqlType) {
+ this.name = name;
+ this.index = index;
+ this.sqlType = sqlType;
+ }
+
+ public void define(SchemaBuilder builder) {
+ builder.addNullable(getName(), COLUMN_TYPE_MAP.get(getSqlType()));
+ }
+
+ public void bind(RowSetLoader loader) {
+ writer = loader.scalar(getName());
+ }
+
+ public abstract void load(Object value);
+ }
+
+ protected static abstract class GenericDefn extends ColumnDefn {
+
+ public GenericDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+ }
+
+ protected static class GenericVarcharDefn extends GenericDefn {
+
+ public GenericVarcharDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setString((String) value);
+ }
+ }
+
+ protected static class GenericBigintDefn extends GenericDefn {
+
+ public GenericBigintDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setLong((Long) value);
+ }
+ }
+
+ protected static class GenericIntegerDefn extends GenericDefn {
+
+ public GenericIntegerDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setInt((Integer) value);
+ }
+ }
+
+ protected static class GenericSmallintDefn extends GenericDefn {
+
+ public GenericSmallintDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setInt((Short) value);
+ }
+ }
+
+ protected static class GenericTinyintDefn extends GenericDefn {
+
+ public GenericTinyintDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setInt((Byte) value);
+ }
+ }
+
+ protected static class GenericDoubleDefn extends GenericDefn {
+
+ public GenericDoubleDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setDouble((Double) value);
+ }
+ }
+
+ protected static class GenericFloatDefn extends GenericDefn {
+
+ public GenericFloatDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setFloat((Float) value);
Review comment:
Two minor suggestions. First, maybe have an abstract scalar class to avoid the repeated casts. The scalar abstract class holds the `ScalarWriter` while the array version holds the `ScalarArrayWriter`.
Second, for scalars, it is fine to call `setObject()` which will do the cast for you. The scalar writers don't do any of the "object parsing" logic that arrays do: they expect the object to be of the correct type.
What that, you'd have:
```java
writer.setObject(value)
```
Because of that, you can have one class for the types that don't need conversion (which seems to be Float, Double, String, etc.) since the implementation would be the same for all of them.
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixReader.java
##########
@@ -0,0 +1,463 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.math.BigDecimal;
+import java.sql.Array;
+import java.sql.Date;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Time;
+import java.sql.Timestamp;
+import java.sql.Types;
+import java.util.Arrays;
+import java.util.Map;
+
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.physical.resultSet.ResultSetLoader;
+import org.apache.drill.exec.physical.resultSet.RowSetLoader;
+import org.apache.drill.exec.record.metadata.SchemaBuilder;
+import org.apache.drill.exec.vector.accessor.ColumnWriter;
+import org.apache.drill.exec.vector.accessor.ScalarWriter;
+import org.apache.drill.shaded.guava.com.google.common.collect.Maps;
+
+public class PhoenixReader {
+
+ private final RowSetLoader writer;
+ private final ColumnDefn[] columns;
+ private final ResultSet results;
+ private long count;
+
+ public PhoenixReader(ResultSetLoader loader, ColumnDefn[] columns, ResultSet results) {
+ this.writer = loader.writer();
+ this.columns = columns;
+ this.results = results;
+ }
+
+ public RowSetLoader getStorage() {
+ return writer;
+ }
+
+ public long getCount() {
+ return count;
+ }
+
+ /**
+ * Fetch and process one row.
+ * @return return true if one row is processed, return false if there is no next row.
+ * @throws SQLException
+ */
+ public boolean processRow() throws SQLException {
+ if (results.next()) {
+ writer.start();
+ for (int index = 0; index < columns.length; index++) {
+ if (columns[index].getSqlType() == Types.ARRAY) {
+ Array result = results.getArray(index + 1);
+ if (result != null) {
+ columns[index].load(result.getArray());
+ }
+ } else {
+ Object result = results.getObject(index + 1);
+ if (result != null) {
+ columns[index].load(result);
+ }
+ }
+ }
+ count++;
Review comment:
The result set loader maintains row and batch counts for you, if you need them.
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixStoragePluginConfig.java
##########
@@ -0,0 +1,141 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.drill.common.PlanStringBuilder;
+import org.apache.drill.common.logical.AbstractSecuredStoragePluginConfig;
+import org.apache.drill.common.logical.security.CredentialsProvider;
+import org.apache.drill.exec.store.security.CredentialProviderUtils;
+import org.apache.drill.exec.store.security.UsernamePasswordCredentials;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+
+@JsonTypeName(PhoenixStoragePluginConfig.NAME)
+public class PhoenixStoragePluginConfig extends AbstractSecuredStoragePluginConfig {
+
+ public static final String NAME = "phoenix";
+ public static final String THIN_DRIVER_CLASS = "org.apache.phoenix.queryserver.client.Driver";
+ public static final String FAT_DRIVER_CLASS = "org.apache.phoenix.jdbc.PhoenixDriver";
+
+ private final String host;
+ private final int port;
+ private final String jdbcURL; // (options) Equal to host + port
+ private final Map<String, Object> props; // (options) See also http://phoenix.apache.org/tuning.html
+
+ @JsonCreator
+ public PhoenixStoragePluginConfig(
+ @JsonProperty("host") String host,
+ @JsonProperty("port") int port,
+ @JsonProperty("username") String username,
+ @JsonProperty("password") String password,
+ @JsonProperty("jdbcURL") String jdbcURL,
+ @JsonProperty("credentialsProvider") CredentialsProvider credentialsProvider,
+ @JsonProperty("props") Map<String, Object> props) {
+ super(CredentialProviderUtils.getCredentialsProvider(username, password, credentialsProvider), credentialsProvider == null);
+ this.host = host;
+ this.port = port == 0 ? 8765 : port;
+ this.jdbcURL = jdbcURL;
+ this.props = props == null ? Collections.emptyMap() : props;
+ }
+
+ @JsonIgnore
+ public UsernamePasswordCredentials getUsernamePasswordCredentials() {
+ return new UsernamePasswordCredentials(credentialsProvider);
+ }
+
+ @JsonProperty("host")
+ public String getHost() {
+ return host;
+ }
+
+ @JsonProperty("port")
+ public int getPort() {
+ return port;
+ }
+
+ @JsonProperty("username")
+ public String getUsername() {
+ if (directCredentials) {
+ return getUsernamePasswordCredentials().getUsername();
+ }
+ return null;
+ }
+
+ @JsonIgnore
+ @JsonProperty("password")
+ public String getPassword() {
+ if (directCredentials) {
+ return getUsernamePasswordCredentials().getPassword();
Review comment:
This can probably just be `getCredentials()` since there are no credentials here other than user name/password.
##########
File path: exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/writer/ScalarArrayWriter.java
##########
@@ -201,12 +218,30 @@ public void setByteArray(byte[] value) {
}
}
+ public void setByteObjectArray(Byte[] value) {
+ for (int i = 0; i < value.length; i++) {
+ final Byte element = value[i];
+ if (element != null) {
+ elementWriter.setInt(element);
+ }
+ }
+ }
+
public void setShortArray(short[] value) {
for (int i = 0; i < value.length; i++) {
elementWriter.setInt(value[i]);
}
}
+ public void setShortObjectArray(Short[] value) {
+ for (int i = 0; i < value.length; i++) {
+ final Short element = value[i];
+ if (element != null) {
+ elementWriter.setInt(element);
Review comment:
Thanks for fixing this. While you're at it, can you fix the existing `setFooObjectArray()` methods that call `setNull()`? Not sure how they got into the code...
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixReader.java
##########
@@ -0,0 +1,463 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.math.BigDecimal;
+import java.sql.Array;
+import java.sql.Date;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Time;
+import java.sql.Timestamp;
+import java.sql.Types;
+import java.util.Arrays;
+import java.util.Map;
+
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.physical.resultSet.ResultSetLoader;
+import org.apache.drill.exec.physical.resultSet.RowSetLoader;
+import org.apache.drill.exec.record.metadata.SchemaBuilder;
+import org.apache.drill.exec.vector.accessor.ColumnWriter;
+import org.apache.drill.exec.vector.accessor.ScalarWriter;
+import org.apache.drill.shaded.guava.com.google.common.collect.Maps;
+
+public class PhoenixReader {
+
+ private final RowSetLoader writer;
+ private final ColumnDefn[] columns;
+ private final ResultSet results;
+ private long count;
+
+ public PhoenixReader(ResultSetLoader loader, ColumnDefn[] columns, ResultSet results) {
+ this.writer = loader.writer();
+ this.columns = columns;
+ this.results = results;
+ }
+
+ public RowSetLoader getStorage() {
+ return writer;
+ }
+
+ public long getCount() {
+ return count;
+ }
+
+ /**
+ * Fetch and process one row.
+ * @return return true if one row is processed, return false if there is no next row.
+ * @throws SQLException
+ */
+ public boolean processRow() throws SQLException {
+ if (results.next()) {
+ writer.start();
+ for (int index = 0; index < columns.length; index++) {
+ if (columns[index].getSqlType() == Types.ARRAY) {
Review comment:
Looking better. Even this if-statement can be removed. Each column should know its index. Then, for the generic scalars (see comment below) you could write:
```java
public void load(Something results) {
scalar.setObject(results.getObject(index))
}
```
Similar for the array load.
Note that the stored index would be for the `result` object to avoid the `+ 1` for every column.
Your loop would then look like this:
```java
for (Something column : columns) {
column.load(results);
}
```
Code can't get much simpler than that.
##########
File path: contrib/storage-phoenix/src/test/resources/hbase-site.xml
##########
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<?xml-stylesheet type="text/xsl" href="configuration.xsl"?>
+<!--
+
+ 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.
+
+-->
+<configuration>
+ <property>
+ <name>hbase.master.start.timeout.localHBaseCluster</name>
+ <value>60000</value>
+ </property>
+ <property>
+ <name>phoenix.schema.isNamespaceMappingEnabled</name>
+ <value>true</value>
+ </property>
+</configuration>
Review comment:
Nit: missing final newline.
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixStoragePlugin.java
##########
@@ -0,0 +1,123 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.io.IOException;
+import java.util.Set;
+
+import javax.sql.DataSource;
+
+import org.apache.calcite.adapter.jdbc.JdbcSchema;
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.calcite.sql.SqlDialectFactoryImpl;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.drill.common.JSONOptions;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.exec.ops.OptimizerRulesContext;
+import org.apache.drill.exec.physical.base.AbstractGroupScan;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.AbstractStoragePlugin;
+import org.apache.drill.exec.store.SchemaConfig;
+import org.apache.drill.exec.store.phoenix.rules.PhoenixConvention;
+import org.apache.drill.shaded.guava.com.google.common.collect.Maps;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+
+public class PhoenixStoragePlugin extends AbstractStoragePlugin {
+
+ private final PhoenixStoragePluginConfig config;
+ private final DataSource dataSource;
+ private final SqlDialect dialect;
+ private final PhoenixConvention convention;
+ private final PhoenixSchemaFactory schemaFactory;
+
+ public PhoenixStoragePlugin(PhoenixStoragePluginConfig config, DrillbitContext context, String name) {
+ super(context, name);
+ this.config = config;
+ this.dataSource = initNoPoolingDataSource(config);
+ this.dialect = JdbcSchema.createDialect(SqlDialectFactoryImpl.INSTANCE, dataSource);
+ this.convention = new PhoenixConvention(dialect, name, this);
+ this.schemaFactory = new PhoenixSchemaFactory(this);
+ }
+
+ @Override
+ public StoragePluginConfig getConfig() {
+ return config;
+ }
+
+ public DataSource getDataSource() {
+ return dataSource;
+ }
+
+ public SqlDialect getDialect() {
+ return dialect;
+ }
+
+ public PhoenixConvention getConvention() {
+ return convention;
+ }
+
+ @Override
+ public boolean supportsRead() {
+ return true;
+ }
+
+ @Override
+ public Set<? extends RelOptRule> getPhysicalOptimizerRules(OptimizerRulesContext optimizerRulesContext) {
+ return convention.getRules();
+ }
+
+ @Override
+ public void registerSchemas(SchemaConfig schemaConfig, SchemaPlus parent) throws IOException {
+ schemaFactory.registerSchemas(schemaConfig, parent);
+ }
+
+ @Override
+ public AbstractGroupScan getPhysicalScan(String userName, JSONOptions selection) throws IOException {
+ PhoenixScanSpec scanSpec = selection.getListWith(context.getLpPersistence().getMapper(), new TypeReference<PhoenixScanSpec>() {});
+ return new PhoenixGroupScan(scanSpec, this);
+ }
+
+ private static DataSource initNoPoolingDataSource(PhoenixStoragePluginConfig config) {
+ // Don't use the pool with the connection
+ PhoenixDataSource dataSource = null;
+ if (StringUtils.isNotBlank(config.getJdbcURL())) {
+ if (!config.getProps().isEmpty()) {
+ dataSource = new PhoenixDataSource(config.getJdbcURL(), config.getProps());
+ } else {
+ dataSource = new PhoenixDataSource(config.getJdbcURL());
+ }
+ } else {
+ if (!config.getProps().isEmpty()) {
+ dataSource = new PhoenixDataSource(config.getHost(), config.getPort(), config.getProps());
+ } else {
+ dataSource = new PhoenixDataSource(config.getHost(), config.getPort());
+ }
+ }
Review comment:
Suggestion:
```java
Map<String, Object> = config.getProps();
if (props == null) {
props = new HashMap<>();
}
```
Then, you can eliminate the various props/no props variations, just always pass the (possibly empty) props. Even better, check if the props version accepts a null props value.
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixReader.java
##########
@@ -0,0 +1,463 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.math.BigDecimal;
+import java.sql.Array;
+import java.sql.Date;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Time;
+import java.sql.Timestamp;
+import java.sql.Types;
+import java.util.Arrays;
+import java.util.Map;
+
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.physical.resultSet.ResultSetLoader;
+import org.apache.drill.exec.physical.resultSet.RowSetLoader;
+import org.apache.drill.exec.record.metadata.SchemaBuilder;
+import org.apache.drill.exec.vector.accessor.ColumnWriter;
+import org.apache.drill.exec.vector.accessor.ScalarWriter;
+import org.apache.drill.shaded.guava.com.google.common.collect.Maps;
+
+public class PhoenixReader {
+
+ private final RowSetLoader writer;
+ private final ColumnDefn[] columns;
+ private final ResultSet results;
+ private long count;
+
+ public PhoenixReader(ResultSetLoader loader, ColumnDefn[] columns, ResultSet results) {
+ this.writer = loader.writer();
+ this.columns = columns;
+ this.results = results;
+ }
+
+ public RowSetLoader getStorage() {
+ return writer;
+ }
+
+ public long getCount() {
+ return count;
+ }
+
+ /**
+ * Fetch and process one row.
+ * @return return true if one row is processed, return false if there is no next row.
+ * @throws SQLException
+ */
+ public boolean processRow() throws SQLException {
+ if (results.next()) {
+ writer.start();
+ for (int index = 0; index < columns.length; index++) {
+ if (columns[index].getSqlType() == Types.ARRAY) {
+ Array result = results.getArray(index + 1);
+ if (result != null) {
+ columns[index].load(result.getArray());
+ }
+ } else {
+ Object result = results.getObject(index + 1);
+ if (result != null) {
+ columns[index].load(result);
+ }
+ }
+ }
+ count++;
+ writer.save();
+ return true;
+ }
+ return false;
+ }
+
+ protected static final Map<Integer, MinorType> COLUMN_TYPE_MAP = Maps.newHashMap();
+
+ static {
+ // text
+ COLUMN_TYPE_MAP.put(Types.VARCHAR, MinorType.VARCHAR);
+ COLUMN_TYPE_MAP.put(Types.CHAR, MinorType.VARCHAR);
+ // numbers
+ COLUMN_TYPE_MAP.put(Types.BIGINT, MinorType.BIGINT);
+ COLUMN_TYPE_MAP.put(Types.INTEGER, MinorType.INT);
+ COLUMN_TYPE_MAP.put(Types.SMALLINT, MinorType.INT);
+ COLUMN_TYPE_MAP.put(Types.TINYINT, MinorType.INT);
+ COLUMN_TYPE_MAP.put(Types.DOUBLE, MinorType.FLOAT8);
+ COLUMN_TYPE_MAP.put(Types.FLOAT, MinorType.FLOAT4);
+ COLUMN_TYPE_MAP.put(Types.DECIMAL, MinorType.VARDECIMAL);
+ // time
+ COLUMN_TYPE_MAP.put(Types.DATE, MinorType.DATE);
+ COLUMN_TYPE_MAP.put(Types.TIME, MinorType.TIME);
+ COLUMN_TYPE_MAP.put(Types.TIMESTAMP, MinorType.TIMESTAMP);
+ // binary
+ COLUMN_TYPE_MAP.put(Types.BINARY, MinorType.VARBINARY); // Raw fixed length byte array. Mapped to byte[].
+ COLUMN_TYPE_MAP.put(Types.VARBINARY, MinorType.VARBINARY); // Raw variable length byte array.
+ // boolean
+ COLUMN_TYPE_MAP.put(Types.BOOLEAN, MinorType.BIT);
+ }
+
+ protected abstract static class ColumnDefn {
+
+ final String name;
+ final int index;
+ final int sqlType;
+ ColumnWriter writer;
+
+ public String getName() {
+ return name;
+ }
+
+ public int getIndex() {
+ return index;
+ }
+
+ public int getSqlType() {
+ return sqlType;
+ }
+
+ public ColumnDefn(String name, int index, int sqlType) {
+ this.name = name;
+ this.index = index;
+ this.sqlType = sqlType;
+ }
+
+ public void define(SchemaBuilder builder) {
+ builder.addNullable(getName(), COLUMN_TYPE_MAP.get(getSqlType()));
+ }
+
+ public void bind(RowSetLoader loader) {
+ writer = loader.scalar(getName());
+ }
+
+ public abstract void load(Object value);
+ }
+
+ protected static abstract class GenericDefn extends ColumnDefn {
+
+ public GenericDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+ }
+
+ protected static class GenericVarcharDefn extends GenericDefn {
+
+ public GenericVarcharDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setString((String) value);
+ }
+ }
+
+ protected static class GenericBigintDefn extends GenericDefn {
+
+ public GenericBigintDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setLong((Long) value);
+ }
+ }
+
+ protected static class GenericIntegerDefn extends GenericDefn {
+
+ public GenericIntegerDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setInt((Integer) value);
+ }
+ }
+
+ protected static class GenericSmallintDefn extends GenericDefn {
+
+ public GenericSmallintDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setInt((Short) value);
+ }
+ }
+
+ protected static class GenericTinyintDefn extends GenericDefn {
+
+ public GenericTinyintDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setInt((Byte) value);
+ }
+ }
+
+ protected static class GenericDoubleDefn extends GenericDefn {
+
+ public GenericDoubleDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setDouble((Double) value);
+ }
+ }
+
+ protected static class GenericFloatDefn extends GenericDefn {
+
+ public GenericFloatDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setFloat((Float) value);
+ }
+ }
+
+ protected static class GenericDecimalDefn extends GenericDefn {
+
+ public GenericDecimalDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setDecimal((BigDecimal) value);
+ }
+ }
+
+ protected static class GenericDateDefn extends GenericDefn {
+
+ public GenericDateDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setDate(((Date) value).toLocalDate());
+ }
+ }
+
+ protected static class GenericTimeDefn extends GenericDefn {
+
+ public GenericTimeDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setTime(((Time) value).toLocalTime());
+ }
+ }
+
+ protected static class GenericTimestampDefn extends GenericDefn {
+
+ public GenericTimestampDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setTimestamp(((Timestamp) value).toInstant());
+ }
+ }
+
+ protected static class GenericBinaryDefn extends GenericDefn {
+
+ public GenericBinaryDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ byte[] byteValue = (byte[]) value;
+ ((ScalarWriter) writer).setBytes(byteValue, byteValue.length);
+ }
+ }
+
+ protected static class GenericBooleanDefn extends GenericDefn {
+
+ public GenericBooleanDefn(String name, int index, int sqlType) {
+ super(name, index, sqlType);
+ }
+
+ @Override
+ public void load(Object value) {
+ ((ScalarWriter) writer).setBoolean((Boolean) value);
+ }
+ }
+
+ protected static abstract class ArrayDefn extends ColumnDefn {
+
+ static final String VARCHAR = "VARCHAR ARRAY";
+ static final String CHAR = "CHAR ARRAY";
+ static final String BIGINT = "BIGINT ARRAY";
+ static final String INTEGER = "INTEGER ARRAY";
+ static final String DOUBLE = "DOUBLE ARRAY";
+ static final String FLOAT = "FLOAT ARRAY";
+ static final String SMALLINT = "SMALLINT ARRAY";
+ static final String TINYINT = "TINYINT ARRAY";
+ static final String BOOLEAN = "BOOLEAN ARRAY";
+
+ final String baseType;
+
+ public ArrayDefn(String name, int index, int sqlType, String baseType) {
+ super(name, index, sqlType);
+ this.baseType = baseType;
+ }
+
+ @Override
+ public void bind(RowSetLoader loader) {
+ writer = loader.array(getName());
+ }
+ }
+
+ protected static class ArrayVarcharDefn extends ArrayDefn {
+
+ public ArrayVarcharDefn(String name, int index, int sqlType, String baseType) {
+ super(name, index, sqlType, baseType);
+ }
+
+ @Override
+ public void define(SchemaBuilder builder) {
+ builder.addArray(getName(), MinorType.VARCHAR);
+ }
+
+ @Override
+ public void load(Object value) {
+ Object[] values = (Object[]) value;
+ writer.setObject(Arrays.copyOf(values, values.length, String[].class));
+ }
+ }
+
+ protected static class ArrayBigintDefn extends ArrayDefn {
+
+ public ArrayBigintDefn(String name, int index, int sqlType, String baseType) {
+ super(name, index, sqlType, baseType);
+ }
+
+ @Override
+ public void define(SchemaBuilder builder) {
+ builder.addArray(getName(), MinorType.BIGINT);
+ }
+
+ @Override
+ public void load(Object value) {
+ Object[] values = (Object[]) value;
+ writer.setObject(Arrays.copyOf(values, values.length, Long[].class));
+ }
+ }
+
+ protected static class ArrayIntegerDefn extends ArrayDefn {
+
+ public ArrayIntegerDefn(String name, int index, int sqlType, String baseType) {
+ super(name, index, sqlType, baseType);
+ }
+
+ @Override
+ public void define(SchemaBuilder builder) {
+ builder.addArray(getName(), MinorType.INT);
+ }
+
+ @Override
+ public void load(Object value) {
+ Object[] values = (Object[]) value;
+ writer.setObject(Arrays.copyOf(values, values.length, Integer[].class));
+ }
+ }
+
+ protected static class ArraySmallintDefn extends ArrayDefn {
+
+ public ArraySmallintDefn(String name, int index, int sqlType, String baseType) {
+ super(name, index, sqlType, baseType);
+ }
+
+ @Override
+ public void define(SchemaBuilder builder) {
+ builder.addArray(getName(), MinorType.SMALLINT);
+ }
+
+ @Override
+ public void load(Object value) {
+ Object[] values = (Object[]) value;
+ writer.setObject(Arrays.copyOf(values, values.length, Short[].class));
+ }
+ }
+
+ protected static class ArrayTinyintDefn extends ArrayDefn {
+
+ public ArrayTinyintDefn(String name, int index, int sqlType, String baseType) {
+ super(name, index, sqlType, baseType);
+ }
+
+ @Override
+ public void define(SchemaBuilder builder) {
+ builder.addArray(getName(), MinorType.TINYINT);
+ }
+
+ @Override
+ public void load(Object value) {
+ Object[] values = (Object[]) value;
+ writer.setObject(Arrays.copyOf(values, values.length, Byte[].class));
+ }
+ }
+
+ protected static class ArrayDoubleDefn extends ArrayDefn {
+
+ public ArrayDoubleDefn(String name, int index, int sqlType, String baseType) {
+ super(name, index, sqlType, baseType);
+ }
+
+ @Override
+ public void define(SchemaBuilder builder) {
+ builder.addArray(getName(), MinorType.FLOAT8);
+ }
+
+ @Override
+ public void load(Object value) {
+ Object[] values = (Object[]) value;
+ writer.setObject(Arrays.copyOf(values, values.length, Double[].class));
+ }
+ }
+
+ protected static class ArrayBooleanDefn extends ArrayDefn {
+
+ public ArrayBooleanDefn(String name, int index, int sqlType, String baseType) {
+ super(name, index, sqlType, baseType);
+ }
+
+ @Override
+ public void define(SchemaBuilder builder) {
+ builder.addArray(getName(), MinorType.BIT);
+ }
+
+ @Override
+ public void load(Object value) {
+ Object[] values = (Object[]) value;
+ writer.setObject(Arrays.copyOf(values, values.length, Boolean[].class));
Review comment:
This is good. We can do better. `setObject()` for an array parses the array type, which is more-or-less the switch statement we wanted to avoid. In order for the parser to work, you had to copy the array, which is a nuisance. So, instead ensure that `writer` is of type `ScalarArrayWriter` and call `setBooleanObjectArray((Boolean[]) value)` instead. (But, it seems that we don't have this method, so it would need to be added...)
Now, it might be that the cast won't work. So, the next change would be to add a `setObjectArray()` method to `ScalarArrayWriter` something like this:
```java
public void setObjectArray(Object[] value) {
for (int i = 0; i < value.length; i++) {
final Object element = value[i];
if (element == null) {
elementWriter.setNull();
} else {
elementWriter.setObject(element);
}
}
}
```
In the above, the `setObject()` call will be implemented by the underlying Boolean writer, which will expect the object to be a `Boolean`.
If either of these works, apply the same change to the other array classes here.
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixStoragePluginConfig.java
##########
@@ -0,0 +1,141 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.drill.common.PlanStringBuilder;
+import org.apache.drill.common.logical.AbstractSecuredStoragePluginConfig;
+import org.apache.drill.common.logical.security.CredentialsProvider;
+import org.apache.drill.exec.store.security.CredentialProviderUtils;
+import org.apache.drill.exec.store.security.UsernamePasswordCredentials;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+
+@JsonTypeName(PhoenixStoragePluginConfig.NAME)
+public class PhoenixStoragePluginConfig extends AbstractSecuredStoragePluginConfig {
+
+ public static final String NAME = "phoenix";
+ public static final String THIN_DRIVER_CLASS = "org.apache.phoenix.queryserver.client.Driver";
+ public static final String FAT_DRIVER_CLASS = "org.apache.phoenix.jdbc.PhoenixDriver";
+
+ private final String host;
+ private final int port;
+ private final String jdbcURL; // (options) Equal to host + port
+ private final Map<String, Object> props; // (options) See also http://phoenix.apache.org/tuning.html
+
+ @JsonCreator
+ public PhoenixStoragePluginConfig(
+ @JsonProperty("host") String host,
+ @JsonProperty("port") int port,
+ @JsonProperty("username") String username,
+ @JsonProperty("password") String password,
+ @JsonProperty("jdbcURL") String jdbcURL,
+ @JsonProperty("credentialsProvider") CredentialsProvider credentialsProvider,
+ @JsonProperty("props") Map<String, Object> props) {
+ super(CredentialProviderUtils.getCredentialsProvider(username, password, credentialsProvider), credentialsProvider == null);
+ this.host = host;
+ this.port = port == 0 ? 8765 : port;
+ this.jdbcURL = jdbcURL;
+ this.props = props == null ? Collections.emptyMap() : props;
+ }
+
+ @JsonIgnore
+ public UsernamePasswordCredentials getUsernamePasswordCredentials() {
+ return new UsernamePasswordCredentials(credentialsProvider);
+ }
+
+ @JsonProperty("host")
+ public String getHost() {
+ return host;
+ }
+
+ @JsonProperty("port")
+ public int getPort() {
+ return port;
+ }
+
+ @JsonProperty("username")
+ public String getUsername() {
+ if (directCredentials) {
+ return getUsernamePasswordCredentials().getUsername();
+ }
+ return null;
+ }
+
+ @JsonIgnore
+ @JsonProperty("password")
+ public String getPassword() {
+ if (directCredentials) {
+ return getUsernamePasswordCredentials().getPassword();
+ }
+ return null;
+ }
+
+ @JsonProperty("jdbcURL")
+ public String getJdbcURL() {
+ return jdbcURL;
+ }
+
+ @JsonProperty("props")
+ public Map<String, Object> getProps() {
+ return props;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (o == this) {
+ return true;
+ }
+ if (o == null || !(o instanceof PhoenixStoragePluginConfig)) {
+ return false;
+ }
+ PhoenixStoragePluginConfig config = (PhoenixStoragePluginConfig) o;
+ // URL first
+ if (StringUtils.isNotBlank(config.getJdbcURL())) {
+ return Objects.equals(this.jdbcURL, config.getJdbcURL());
+ }
+ // Then the host and port
+ return Objects.equals(this.host, config.getHost()) && Objects.equals(this.port, config.getPort());
+ }
+
+ @Override
+ public int hashCode() {
+ if (StringUtils.isNotBlank(jdbcURL)) {
+ return Objects.hash(jdbcURL);
+ }
+ return Objects.hash(host, port);
+ }
+
+ @Override
+ public String toString() {
+ return new PlanStringBuilder(PhoenixStoragePluginConfig.NAME)
+ .field("driverName", THIN_DRIVER_CLASS)
+ .field("host", host)
+ .field("port", port)
+ .field("jdbcURL", jdbcURL)
+ .field("props", props)
Review comment:
Should this include the user name? And an obfuscated password? That is, show, say, "*****" if the password is set. (Show the same number of asterisks regardless of password length.)
##########
File path: contrib/storage-phoenix/src/main/resources/logback-test.xml.bak
##########
@@ -0,0 +1,49 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!--
+ 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.
+-->
+<configuration>
Review comment:
Did you check if this is actually needed? AFAIK, Logback will see all these files on the class path, and will load only one.
Also, `logback-test.xml` probably should not be in the `main/resources`, else Logback will load it even in production.
If this was copy-pasted from another extension plugin, then that one is probably also wrong.
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixStoragePluginConfig.java
##########
@@ -0,0 +1,141 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.drill.common.PlanStringBuilder;
+import org.apache.drill.common.logical.AbstractSecuredStoragePluginConfig;
+import org.apache.drill.common.logical.security.CredentialsProvider;
+import org.apache.drill.exec.store.security.CredentialProviderUtils;
+import org.apache.drill.exec.store.security.UsernamePasswordCredentials;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+
+@JsonTypeName(PhoenixStoragePluginConfig.NAME)
+public class PhoenixStoragePluginConfig extends AbstractSecuredStoragePluginConfig {
+
+ public static final String NAME = "phoenix";
+ public static final String THIN_DRIVER_CLASS = "org.apache.phoenix.queryserver.client.Driver";
+ public static final String FAT_DRIVER_CLASS = "org.apache.phoenix.jdbc.PhoenixDriver";
+
+ private final String host;
+ private final int port;
+ private final String jdbcURL; // (options) Equal to host + port
+ private final Map<String, Object> props; // (options) See also http://phoenix.apache.org/tuning.html
+
+ @JsonCreator
+ public PhoenixStoragePluginConfig(
+ @JsonProperty("host") String host,
+ @JsonProperty("port") int port,
+ @JsonProperty("username") String username,
Review comment:
Nit: a quick search of the sources suggests the Drill convention here is `"userName"` (upper case "N").
##########
File path: contrib/storage-phoenix/src/main/resources/bootstrap-storage-plugins.json
##########
@@ -0,0 +1,14 @@
+{
+ "storage": {
+ "phoenix": {
+ "type": "phoenix",
+ "jdbcURL": "jdbc:phoenix:thin:url=http://the.queryserver.hostname:8765;serialization=PROTOBUF",
Review comment:
Just to clarify, can this be:
```json
"jdbcURL": "jdbc:phoenix:thin:url=http://localhost:8765;serialization=PROTOBUF",
```
Actually, let's think about this a bit more. You're using JDBC with the thin client. You require the `PROTOBUF` serialization. Looks like the config allows a host and port, and the reader code handles that case. So, can the JSON instead be:
```json
"host": "localhost",
"port": 8765,
```
It is much more likely people will get the above right: they only have to change the host field. No chance to accidentally change the wrong bits of the URL.
Leave the URL only for an advanced case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Z0ltrix edited a comment on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
Z0ltrix edited a comment on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-959753259
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-958973388
@luocooong Thanks for this PR. This will be a really great addition to Drill! I wanted to suggest that since there isn't a testcontainer for Apache Phoenix, that it might be a better approach to use the Phoenix test classes. Here's one that I found: https://github.com/apache/phoenix/blob/master/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
Maybe take a look and see what you think. From my recollection, this is what we had to do with the Splunk connector.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on a change in pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
luocooong commented on a change in pull request #2332:
URL: https://github.com/apache/drill/pull/2332#discussion_r758509422
##########
File path: contrib/storage-phoenix/src/main/resources/logback-test.xml.bak
##########
@@ -0,0 +1,49 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!--
+ 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.
+-->
+<configuration>
Review comment:
The original plan was to delete it before merging the PR, but now I have deleted it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] paul-rogers commented on a change in pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
paul-rogers commented on a change in pull request #2332:
URL: https://github.com/apache/drill/pull/2332#discussion_r760821424
##########
File path: contrib/storage-phoenix/README.md
##########
@@ -0,0 +1,98 @@
+# [DRILL-7863](https://issues.apache.org/jira/browse/DRILL-7863): Add Storage Plugin for Apache Phoenix
+
+## Description
+
+ Phoenix say : "We put the SQL back in NoSQL",<br/>
+ Drill call : "We use the SQL to cross almost all the file systems and storage engines",<br/>
+ "Cheers !", users said.
+
+## Documentation
+
+Features :
+
+ - Full support for Enhanced Vector Framework.
+
+ - Tested in phoenix 4.14 and 5.1.2.
+
+ - Support the array data type.
+
+ - Support the pushdown (Project, Limit, Filter, Aggregate, Join, CrossJoin, Join_Filter, GroupBy, Distinct and more).
+
+ - Use the PQS client (6.0).
+
+Related Information :
+
+ 1. PHOENIX-6398: Returns uniform SQL dialect in calcite for the PQS
+
+ 2. PHOENIX-6582: Bump default HBase version to 2.3.7 and 2.4.8
+
+ 3. PHOENIX-6605, PHOENIX-6606 and PHOENIX-6607.
+
+ 4. DRILL-8060, DRILL-8061 and DRILL-8062.
+
+ 5. [QueryServer 6.0.0-drill-r1](https://github.com/luocooong/phoenix-queryserver/releases/tag/6.0.0-drill-r1)
+
+## Testing
+
+ The test framework of phoenix queryserver required the Hadoop 3, but exist `PHOENIX-5993` and `HBASE-22394` :
+
+```
+" The HBase PMC does not release multiple artifacts for both Hadoop2 and Hadoop3 support at the current time.
+Current HBase2 releases still compile against Hadoop2 by default, and using Hadoop 3 against HBase2
+requires a recompilation of HBase because of incompatible changes between Hadoop2 and Hadoop3. "
+```
+
+### Recommended Practices
+
+ 1. Download HBase 2.4.2 sources and rebuild with Hadoop 3.
+
+ 2. Remove the `Ignore` annotation in `PhoenixTestSuite.java`.
+
+ 3. Go to the phoenix root folder and run test.
+
+### To Add Features
+
+ - Don't forget to add a test function to the test class.
+
+ - If a new test class is added, please declare it in the `PhoenixTestSuite` class.
+
+### Play in CLI
+
+```sql
Review comment:
Nit: `text`. All the tables and what-not will throw off the SQL formatter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on a change in pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
luocooong commented on a change in pull request #2332:
URL: https://github.com/apache/drill/pull/2332#discussion_r758505941
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixStoragePluginConfig.java
##########
@@ -0,0 +1,141 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.drill.common.PlanStringBuilder;
+import org.apache.drill.common.logical.AbstractSecuredStoragePluginConfig;
+import org.apache.drill.common.logical.security.CredentialsProvider;
+import org.apache.drill.exec.store.security.CredentialProviderUtils;
+import org.apache.drill.exec.store.security.UsernamePasswordCredentials;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+
+@JsonTypeName(PhoenixStoragePluginConfig.NAME)
+public class PhoenixStoragePluginConfig extends AbstractSecuredStoragePluginConfig {
+
+ public static final String NAME = "phoenix";
+ public static final String THIN_DRIVER_CLASS = "org.apache.phoenix.queryserver.client.Driver";
+ public static final String FAT_DRIVER_CLASS = "org.apache.phoenix.jdbc.PhoenixDriver";
+
+ private final String host;
+ private final int port;
+ private final String jdbcURL; // (options) Equal to host + port
+ private final Map<String, Object> props; // (options) See also http://phoenix.apache.org/tuning.html
+
+ @JsonCreator
+ public PhoenixStoragePluginConfig(
+ @JsonProperty("host") String host,
+ @JsonProperty("port") int port,
+ @JsonProperty("username") String username,
Review comment:
As a side note, What is the difference between `opUserName` and `queryUserName`?
```java
opUserName = scan.getUserName();
queryUserName = negotiator.context().getFragmentContext().getQueryUserName();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Z0ltrix commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
Z0ltrix commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-960809785
>
>
> > Of course this PR is great even without working Impersonation, but for enterprise its necessary to let the enduser run the query against the storage.
> > The fact that drill impersonates the proxyUser correctly against HDFS and HBase let me hope that this is also feasable for phoenix :)
>
> Hi @Z0ltrix. I believe that Drill uses the native support for impersonation that is built into Hadoop when it impersonates to HDFS, Hive, HBase (and MapR-DB through whatever is built into that). No other storage plugins support any impersonation yet. Also there is no support for propagating a Kerberos authn context (a TGT I guess?) over the hop from Drill to the external data source. But... we want it to be there! We are in a planning phase and talking to security guys and we want to start coding on this in November. Your input is very valuable so we'd like to keep checking in with you as we go.
>
> cc @cgivre
Hi @dzamo ,
phoenix uses keytab and other stuff within the connection string: https://phoenix.apache.org/#connStr isnt this what we need?
Or, the other aspect... use code from PQS for the connection to phoenix... they use some sort of proxyUser from org.apache.hadoop.security.UserGroupInformation as well: https://github.com/apache/phoenix-queryserver/blob/master/phoenix-queryserver/src/main/java/org/apache/phoenix/queryserver/server/QueryServer.java#L553
Regards,
Christian
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-959421590
@cgivre Thanks for the information. Actually, I have already tried to use the PQS test framework into Drill. Unfortunately, the Hadoop test dependencies (2.7) and HBase test dependencies (1.4) caused a lot of conflict (Drill use Hadoop 3.2 and HBase 2.4). That's one of the reasons I chose to make a container image.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] dzamo commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
dzamo commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-983644114
Dear PR author and reviewers.
This is a generic message to say that we would like to merge this PR in time for the 1.20 release. Currently we're targeting a master branch freeze date of 2021-12-10 (10 Dec). Please strive to complete development and review by this time, or indicate that the PR will need more time (and how much).
Thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] dzamo commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
dzamo commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-960802212
> >
>
> @dzamo I was under the impression that the vault work we did earlier actually did enable impersonation for most other storage plugins. The key component was the creation of the `credentialProvider` object which allowed that. Maybe @vvysotskyi could weigh in here as well.
@cgivre @vvysotskyi Would be great to hear from him but my understanding is that what we achieved there was to remove the hardcoded credentials of what is still a shared service account from the storage config JSON. We didn't go so far as to impersonate the active Drill user.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] dzamo commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
dzamo commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-960776463
> Of course this PR is great even without working Impersonation, but for enterprise its necessary to let the enduser run the query against the storage.
>
> The fact that drill impersonates the proxyUser correctly against HDFS and HBase let me hope that this is also feasable for phoenix :)
Hi @Z0ltrix. I believe that Drill uses the native support for impersonation that is built into Hadoop when it impersonates to HDFS, Hive, HBase (and MapR-DB through whatever is built into that). No other storage plugins support any impersonation yet. Also there is no support for propagating a Kerberos authn context (a TGT I guess?) over the hop from Drill to the external data source. But... we want it to be there! We are in a planning phase and talking to security guys and we want to start coding on this in November. Your input is very valuable so we'd like to keep checking in with you as we go.
cc @cgivre
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Z0ltrix commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
Z0ltrix commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-960959540
>
>
> > Hi @dzamo , phoenix uses keytab and other stuff within the connection string: https://phoenix.apache.org/#connStr isnt this what we need?
> > Or, the other aspect... use code from PQS for the connection to phoenix... they use some sort of proxyUser from org.apache.hadoop.security.UserGroupInformation as well: https://github.com/apache/phoenix-queryserver/blob/master/phoenix-queryserver/src/main/java/org/apache/phoenix/queryserver/server/QueryServer.java#L553
> > Regards, Christian
>
> @Z0ltrix Okaaay. I'd forgotten that Phoenix lives on top of Hadoop anyway. So if you've already enabled Hadoop's impersonation using the needed `proxyuser` settings then maybe all that we need here is to tack a `doAs` onto the JDBC URL you already have working (which might already include some Kerberos bits).
>
> ```
> jdbc:phoenix:thin:url=http://localhost:8765?doAs=alice
> ```
>
> See https://phoenix.apache.org/server.html#Impersonation. @luocooong this should be reasonably easy, let me know if I can help with anything. We should imitate the behaviour of storage-hive which also contains "doAs". In my head it's something like this farcically simplified pseudocode:
>
> ```
> if (config.getOption('drill.exec.impersonation.enabled') == true) {
> phoenixJdbcUrl = phoenixJdbcUrl + "?doAs=" + activeDrillUser;
> }
> ```
Hi @dzamo,
exaclty... as described in the original Issue #2296 we have the following connection String for PQS
`jdbc:phoenix:thin:url=http://localhost:8765;serialization=PROTOBUF;authentication=SPNEGO;principal=drill/bit@MYCLUSTER;keytab=/etc/hadoop/conf/drill.keytab`
But this will not work with native phoenix connection, because there is no doAs parameter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] lgtm-com[bot] commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-980850416
This pull request **introduces 1 alert** when merging a011dd80e7319f1fa9f268354c79598ba12da1e3 into 5fb79a17b14c59ef9114e4306bfdbabf315a591b - [view on LGTM.com](https://lgtm.com/projects/g/apache/drill/rev/pr-b950dbe80d606d5b19ec8cdf4e2cbee4823c5f6a)
**new alerts:**
* 1 for Unused format argument
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-982640275
@paul-rogers @cgivre Thanks for your support. This plugin is getting better with your help. Could you please take a look again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-959421590
@cgivre Thanks for the information. Actually, I have already tried to use the PQS test framework into Drill. Unfortunately, the Hadoop test dependencies (2.7) and HBase test dependencies (1.4) caused a lot of conflict (Drill use Hadoop 3.2 and HBase 2.4). That's one of the reasons I chose to make a container image.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Z0ltrix edited a comment on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
Z0ltrix edited a comment on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-959753259
> @cgivre Thanks for the information. Actually, I have already tried to use the PQS test framework into Drill. Unfortunately, the Hadoop test dependencies (2.7) and HBase test dependencies (1.4) caused a lot of conflict (Drill use Hadoop 3.2 and HBase 2.4). That's one of the reasons I chose to make a container image.
Hi @luocooong ,
as you can see at the project downloads page, phoenix is shipped in several version-combinations.
For the newest phoenix version > 5 its only build against hbase 2.x and hadoop 3.x
https://github.com/apache/phoenix/blob/master/pom.xml#L78
In fact that hbase 2.3.7 is the current stable release and it recommends hadoop 2.10.0 or 3.1.2, and Drill is already on hadoop 3, i would go with that constellation.
https://github.com/apache/hbase/blob/rel/2.3.7/pom.xml#L1349
phoenix == 5.1.2
PQS == 6.0.0
hbase == 2.3.7
hadoop >= 3.1.2
Hope these thoughts help
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on a change in pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2332:
URL: https://github.com/apache/drill/pull/2332#discussion_r761238385
##########
File path: contrib/storage-phoenix/README.md
##########
@@ -0,0 +1,98 @@
+# [DRILL-7863](https://issues.apache.org/jira/browse/DRILL-7863): Add Storage Plugin for Apache Phoenix
+
+## Description
+
+ Phoenix say : "We put the SQL back in NoSQL",<br/>
+ Drill call : "We use the SQL to cross almost all the file systems and storage engines",<br/>
+ "Cheers !", users said.
+
+## Documentation
+
+Features :
+
+ - Full support for Enhanced Vector Framework.
+
+ - Tested in phoenix 4.14 and 5.1.2.
+
+ - Support the array data type.
+
+ - Support the pushdown (Project, Limit, Filter, Aggregate, Join, CrossJoin, Join_Filter, GroupBy, Distinct and more).
+
+ - Use the PQS client (6.0).
+
+Related Information :
+
+ 1. PHOENIX-6398: Returns uniform SQL dialect in calcite for the PQS
+
+ 2. PHOENIX-6582: Bump default HBase version to 2.3.7 and 2.4.8
+
+ 3. PHOENIX-6605, PHOENIX-6606 and PHOENIX-6607.
+
+ 4. DRILL-8060, DRILL-8061 and DRILL-8062.
+
+ 5. [QueryServer 6.0.0-drill-r1](https://github.com/luocooong/phoenix-queryserver/releases/tag/6.0.0-drill-r1)
+
+## Testing
+
+ The test framework of phoenix queryserver required the Hadoop 3, but exist `PHOENIX-5993` and `HBASE-22394` :
+
+```
+" The HBase PMC does not release multiple artifacts for both Hadoop2 and Hadoop3 support at the current time.
+Current HBase2 releases still compile against Hadoop2 by default, and using Hadoop 3 against HBase2
+requires a recompilation of HBase because of incompatible changes between Hadoop2 and Hadoop3. "
+```
+
+### Recommended Practices
+
+ 1. Download HBase 2.4.2 sources and rebuild with Hadoop 3.
+
+ 2. Remove the `Ignore` annotation in `PhoenixTestSuite.java`.
+
+ 3. Go to the phoenix root folder and run test.
+
+### To Add Features
+
+ - Don't forget to add a test function to the test class.
+
+ - If a new test class is added, please declare it in the `PhoenixTestSuite` class.
+
+### Play in CLI
+
+```sql
Review comment:
The `sql` keyword id fine. Using a lot of tables in the docs can throw things off.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on a change in pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2332:
URL: https://github.com/apache/drill/pull/2332#discussion_r761251990
##########
File path: contrib/storage-phoenix/README.md
##########
@@ -0,0 +1,98 @@
+# [DRILL-7863](https://issues.apache.org/jira/browse/DRILL-7863): Add Storage Plugin for Apache Phoenix
+
+## Description
+
+ Phoenix say : "We put the SQL back in NoSQL",<br/>
+ Drill call : "We use the SQL to cross almost all the file systems and storage engines",<br/>
+ "Cheers !", users said.
+
+## Documentation
+
Review comment:
@luocooong
Thank you so much for this submission. I don't think I could add much more than @paul-rogers did for the code review.
I do have some suggestions for the docs however. Could you please add a section about how to configure Drill to connect to Phoenix and perhaps a demo config?
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Z0ltrix commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
Z0ltrix commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-943135149
Hi @luocooong,
great work! Looking forward to use this storage plugin.
Have you thought about impersonation for this?
https://phoenix.apache.org/server.html#Impersonation explains it for pqs with doAs parameter.
For fat client i´m not sure...
Maybe it has to be done manually, like in the pqs code: https://github.com/apache/phoenix-queryserver/blob/master/phoenix-queryserver/src/main/java/org/apache/phoenix/queryserver/server/QueryServer.java#L553
Do you think its possible to add this?
Regards,
Christian
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-943435016
@Z0ltrix Thanks for the suggestion. In the #2296 , you came up with one good idea, because I might also need it. The key point is that, how can we pass the user name dynamically, on-demand ? Since the last reply, I found a little useful docs : [Submitting Queries from the REST API with Impersonation](https://drill.apache.org/docs/submitting-queries-from-the-rest-api-when-impersonation-is-enabled-and-authentication-is-disabled/) .
All right, that's progress. Do you mind if I complete this (pull request) to-do list before work on the impersonation feature ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] lgtm-com[bot] commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-944902469
This pull request **introduces 1 alert** when merging 3eeb5024df1f2e597a6e023a73235d8874785cb0 into 0c9451e6720e5028e1187067cc6d1957ff998bef - [view on LGTM.com](https://lgtm.com/projects/g/apache/drill/rev/pr-2656c660810b502c2182dde88b74accb84c1b550)
**new alerts:**
* 1 for Potential database resource leak
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] dzamo commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
dzamo commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-960890029
> Hi @dzamo , phoenix uses keytab and other stuff within the connection string: https://phoenix.apache.org/#connStr isnt this what we need?
>
> Or, the other aspect... use code from PQS for the connection to phoenix... they use some sort of proxyUser from org.apache.hadoop.security.UserGroupInformation as well: https://github.com/apache/phoenix-queryserver/blob/master/phoenix-queryserver/src/main/java/org/apache/phoenix/queryserver/server/QueryServer.java#L553
>
> Regards, Christian
@Z0ltrix Okaaay. I'd forgotten that Phoenix lives on top of Hadoop anyway. So if you've already enabled Hadoop's impersonation using the needed `proxyuser` settings then maybe all that we need here is to tack a `doAs` onto the JDBC URL you already have working (which might already include some Kerberos bits).
```
jdbc:phoenix:thin:url=http://localhost:8765?doAs=alice
```
See https://phoenix.apache.org/server.html#Impersonation. @luocooong this should be reasonably easy, let me know if I can help with anything...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-959421590
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-943446349
> @Z0ltrix Thanks for the suggestion. In the #2296 , you came up with one good idea, because I might also need it. The key point is that, how can we pass the user name dynamically, on-demand ? Since the last reply, I found a little useful docs : [Submitting Queries from the REST API with Impersonation](https://drill.apache.org/docs/submitting-queries-from-the-rest-api-when-impersonation-is-enabled-and-authentication-is-disabled/) .
> All right, that's progress. Do you mind if I complete this (pull request) to-do list before work on the impersonation feature ?
@luocooong
I'd actually suggest you take a look at the docs for the credential provider (https://github.com/apache/drill/blob/master/docs/dev/PluginCredentialsProvider.md#developer-notes). It's actually really straightforward to add.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong merged pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
luocooong merged pull request #2332:
URL: https://github.com/apache/drill/pull/2332
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Z0ltrix edited a comment on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
Z0ltrix edited a comment on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-959753259
> @cgivre Thanks for the information. Actually, I have already tried to use the PQS test framework into Drill. Unfortunately, the Hadoop test dependencies (2.7) and HBase test dependencies (1.4) caused a lot of conflict (Drill use Hadoop 3.2 and HBase 2.4). That's one of the reasons I chose to make a container image.
Hi @luocooong ,
as you can see at the project downloads page, phoenix is shipped in several version-combinations.
For the newest phoenix version > 5 its only build against hbase 2.x and hadoop 3.x
https://github.com/apache/phoenix/blob/master/pom.xml#L78
In fact that hbase 2.3.7 is the current stable release and it recommends hadoop 2.10.0 or 3.1.2, and Drill is already on hadoop 3, i would go with that constellation.
https://github.com/apache/hbase/blob/rel/2.3.7/pom.xml#L1349
phoenix == 5.1.2
PQS == 6.0.0
hbase == 2.3.7
hadoop >= 3.1.2
Hope these thoughts help
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-960795904
> > Of course this PR is great even without working Impersonation, but for enterprise its necessary to let the enduser run the query against the storage.
> > The fact that drill impersonates the proxyUser correctly against HDFS and HBase let me hope that this is also feasable for phoenix :)
>
> Hi @Z0ltrix. I believe that Drill uses the native support for impersonation that is built into Hadoop when it impersonates to HDFS, Hive, HBase (and MapR-DB through whatever is built into that). No other storage plugins support any impersonation yet. Also there is no support for propagating a Kerberos authn context (a TGT I guess?) over the hop from Drill to the external data source. But... we want it to be there! We are in a planning phase and talking to security guys and we want to start coding on this in November. Your input is very valuable so we'd like to keep checking in with you as we go.
>
> cc @cgivre
@dzamo I was under the impression that the vault work we did earlier actually did enable impersonation for most other storage plugins. The key component was the creation of the `credentialProvider` object which allowed that. Maybe @vvysotskyi could weigh in here as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] paul-rogers commented on a change in pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
paul-rogers commented on a change in pull request #2332:
URL: https://github.com/apache/drill/pull/2332#discussion_r757120210
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixGroupScan.java
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.util.List;
+import java.util.Objects;
+
+import org.apache.drill.common.PlanStringBuilder;
+import org.apache.drill.common.exceptions.ExecutionSetupException;
+import org.apache.drill.common.expression.SchemaPath;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.exec.physical.PhysicalOperatorSetupException;
+import org.apache.drill.exec.physical.base.AbstractGroupScan;
+import org.apache.drill.exec.physical.base.GroupScan;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.base.ScanStats;
+import org.apache.drill.exec.physical.base.ScanStats.GroupScanProperty;
+import org.apache.drill.exec.physical.base.SubScan;
+import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
+import org.apache.drill.exec.store.StoragePluginRegistry;
+
+import com.fasterxml.jackson.annotation.JacksonInject;
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+
+@JsonTypeName("phoenix-scan")
+public class PhoenixGroupScan extends AbstractGroupScan {
+
+ private final String sql;
+ private final List<SchemaPath> columns;
+ private final PhoenixScanSpec scanSpec;
+ private final double rows;
+ private final ScanStats scanStats;
+ private final PhoenixStoragePlugin plugin;
+
+ private int hashCode;
+
+ @JsonCreator
+ public PhoenixGroupScan(
+ @JsonProperty("sql") String sql,
+ @JsonProperty("columns") List<SchemaPath> columns,
+ @JsonProperty("scanSpec") PhoenixScanSpec scanSpec,
+ @JsonProperty("rows") double rows,
+ @JsonProperty("config") PhoenixStoragePluginConfig config,
+ @JacksonInject StoragePluginRegistry plugins) {
+ super("no-user");
+ this.sql = sql;
+ this.columns = columns;
+ this.scanSpec = scanSpec;
+ this.rows = rows;
+ this.scanStats = computeScanStats();
+ this.plugin = plugins.resolve(config, PhoenixStoragePlugin.class);
+ }
+
+ public PhoenixGroupScan(PhoenixScanSpec scanSpec, PhoenixStoragePlugin plugin) {
+ super("no-user");
+ this.sql = scanSpec.getSql();
+ this.columns = ALL_COLUMNS;
+ this.scanSpec = scanSpec;
+ this.rows = 100;
Review comment:
The row count is used, I believe, to plan joins. Is 100 a good estimate? A number this lows suggests to Drill that it can ship the results to all nodes as part of a broadcast join. If the actual number of rows is 1M or 100M, that will have turned out to be a poor choice.
No good solution here: you actually don't know the number of rows...
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixScanBatchCreator.java
##########
@@ -0,0 +1,93 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.util.List;
+
+import org.apache.drill.common.exceptions.ChildErrorContext;
+import org.apache.drill.common.exceptions.ExecutionSetupException;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.common.types.Types;
+import org.apache.drill.exec.ops.ExecutorFragmentContext;
+import org.apache.drill.exec.physical.impl.BatchCreator;
+import org.apache.drill.exec.physical.impl.scan.framework.ManagedReader;
+import org.apache.drill.exec.physical.impl.scan.framework.ManagedScanFramework;
+import org.apache.drill.exec.physical.impl.scan.framework.ManagedScanFramework.ReaderFactory;
+import org.apache.drill.exec.physical.impl.scan.framework.ManagedScanFramework.ScanFrameworkBuilder;
+import org.apache.drill.exec.physical.impl.scan.framework.SchemaNegotiator;
+import org.apache.drill.exec.record.CloseableRecordBatch;
+import org.apache.drill.exec.record.RecordBatch;
+import org.apache.drill.exec.server.options.OptionManager;
+
+public class PhoenixScanBatchCreator implements BatchCreator<PhoenixSubScan> {
+
+ @Override
+ public CloseableRecordBatch getBatch(ExecutorFragmentContext context, PhoenixSubScan subScan, List<RecordBatch> children) throws ExecutionSetupException {
+ try {
+ ScanFrameworkBuilder builder = createBuilder(context.getOptions(), subScan);
+ return builder.buildScanOperator(context, subScan);
+ } catch (UserException e) {
+ throw e;
+ } catch (Throwable e) {
+ throw new ExecutionSetupException(e);
+ }
+ }
+
+ private ScanFrameworkBuilder createBuilder(OptionManager options, PhoenixSubScan subScan) {
+ ScanFrameworkBuilder builder = new ScanFrameworkBuilder();
+ builder.projection(subScan.getColumns());
+ builder.setUserName(subScan.getUserName());
+ // Phoenix reader
+ ReaderFactory readerFactory = new PhoenixReaderFactory(subScan);
+ builder.setReaderFactory(readerFactory);
+ builder.nullType(Types.optional(MinorType.VARCHAR));
+ // Add custom error context
+ builder.errorContext(new ChildErrorContext(builder.errorContext()) {
+ @Override
+ public void addContext(UserException.Builder builder) {
+ builder.addContext("Database : ", subScan.getScanSpec().getDbName());
+ builder.addContext("Table : ", subScan.getScanSpec().getTableName());
+ }
+ });
+
+ return builder;
+ }
+
+ private static class PhoenixReaderFactory implements ReaderFactory {
+
+ private final PhoenixSubScan subScan;
+ private int count = 0;
Review comment:
Nit: initializer not needed: default is already 0.
##########
File path: contrib/storage-phoenix/src/main/resources/bootstrap-storage-plugins.json
##########
@@ -0,0 +1,14 @@
+{
+ "storage": {
+ "phoenix": {
+ "type": "phoenix",
+ "jdbcURL": "jdbc:phoenix:thin:url=http://the.queryserver.hostname:8765;serialization=PROTOBUF",
Review comment:
Should this point to localhost with the default port? To allow the simplest config to work out-of-the-box?
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixBatchReader.java
##########
@@ -0,0 +1,370 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.math.BigDecimal;
+import java.sql.Array;
+import java.sql.Connection;
+import java.sql.Date;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Time;
+import java.sql.Timestamp;
+import java.sql.Types;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.drill.common.AutoCloseables;
+import org.apache.drill.common.exceptions.CustomErrorContext;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.expression.SchemaPath;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.physical.impl.scan.framework.ManagedReader;
+import org.apache.drill.exec.physical.impl.scan.framework.SchemaNegotiator;
+import org.apache.drill.exec.physical.resultSet.RowSetLoader;
+import org.apache.drill.exec.record.metadata.SchemaBuilder;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+import org.apache.drill.exec.vector.accessor.ColumnWriter;
+import org.apache.drill.exec.vector.accessor.ScalarWriter;
+import org.apache.drill.shaded.guava.com.google.common.base.Stopwatch;
+import org.apache.drill.shaded.guava.com.google.common.collect.Maps;
+import org.slf4j.LoggerFactory;
+
+public class PhoenixBatchReader implements ManagedReader<SchemaNegotiator> {
+
+ private static final org.slf4j.Logger logger = LoggerFactory.getLogger(PhoenixBatchReader.class);
+
+ private final PhoenixSubScan subScan;
+ private CustomErrorContext errorContext;
+ private PhoenixReader reader;
+ private Connection conn;
+ private PreparedStatement pstmt;
+ private ResultSet rs;
+ private ResultSetMetaData meta;
+ private ColumnDefn[] columns;
+ private Stopwatch watch;
+ private int count = 0;
+
+ public PhoenixBatchReader(PhoenixSubScan subScan) {
+ this.subScan = subScan;
+ }
+
+ @Override
+ public boolean open(SchemaNegotiator negotiator) {
+ try {
+ errorContext = negotiator.parentErrorContext();
+ conn = subScan.getPlugin().getDataSource().getConnection();
+ pstmt = conn.prepareStatement(subScan.getSql());
+ rs = pstmt.executeQuery();
+ meta = pstmt.getMetaData();
+ } catch (SQLException e) {
+ throw UserException
+ .dataReadError(e)
+ .message("Failed to execute the phoenix sql query. " + e.getMessage())
+ .build(logger);
+ }
+ try {
+ negotiator.tableSchema(defineMetadata(), true);
+ reader = new PhoenixReader(negotiator.build());
+ bindColumns(reader.getStorage());
+ } catch (SQLException e) {
+ throw UserException
+ .dataReadError(e)
+ .message("Failed to get type of columns from metadata. " + e.getMessage())
+ .build(logger);
+ }
+ watch = Stopwatch.createStarted();
+ return true;
+ }
+
+ @Override
+ public boolean next() {
+ try {
+ while (rs.next()) {
+ { // TODO refactor this to PhoenixReader
+ reader.getStorage().start();
+ for (int index = 0; index < columns.length; index++) {
+ if (columns[index].getSqlType() == Types.ARRAY) {
+ Array result = rs.getArray(index + 1);
+ if (result != null) {
+ columns[index].load(result.getArray());
+ }
+ } else {
+ columns[index].load(rs.getObject(index + 1));
+ }
+ }
+ count++;
+ reader.getStorage().save();
+ }
+ if (reader.getStorage().isFull()) { // batch full but not reached the EOF
+ return true;
+ }
+ }
+ } catch (SQLException e) {
+ throw UserException
+ .dataReadError(e)
+ .message("Failed to get the data from the result set. " + e.getMessage())
+ .build(logger);
+ }
+ watch.stop();
+ logger.debug("Phoenix fetch total record numbers : {}", count);
+ return false; // the EOF is reached.
+ }
+
+ @Override
+ public void close() {
+ count = reader.getStorage().loader().batchCount();
+ logger.debug("Phoenix fetch batch size : {}, took {} ms. ", count, watch.elapsed(TimeUnit.MILLISECONDS));
+ AutoCloseables.closeSilently(rs, pstmt, conn);
+ }
+
+ private TupleMetadata defineMetadata() throws SQLException {
+ List<SchemaPath> cols = subScan.getColumns();
+ columns = new ColumnDefn[cols.size()];
+ SchemaBuilder builder = new SchemaBuilder();
+ for (int index = 0; index < cols.size(); index++) {
+ int sqlType = meta.getColumnType(index + 1); // column the first column is 1
+ String columnName = cols.get(index).rootName();
+ columns[index] = makeColumn(columnName, sqlType, meta.getColumnTypeName(index + 1), index);
+ columns[index].define(builder);
+ }
+ return builder.buildSchema();
+ }
+
+ private ColumnDefn makeColumn(String name, int sqlType, String baseType, int index) {
+ if (sqlType == Types.ARRAY) {
+ return new ArrayDefn(name, sqlType, baseType, index);
+ }
+ return new GenericDefn(name, sqlType, index);
+ }
+
+ private void bindColumns(RowSetLoader loader) {
+ for (int i = 0; i < columns.length; i++) {
+ columns[i].bind(loader);
+ }
+ }
+
+ protected static final Map<Integer, MinorType> COLUMN_TYPE_MAP = Maps.newHashMap();
+
+ static {
+ // text
+ COLUMN_TYPE_MAP.put(Types.VARCHAR, MinorType.VARCHAR);
+ COLUMN_TYPE_MAP.put(Types.CHAR, MinorType.VARCHAR);
+ // numbers
+ COLUMN_TYPE_MAP.put(Types.BIGINT, MinorType.BIGINT);
+ COLUMN_TYPE_MAP.put(Types.INTEGER, MinorType.INT);
+ COLUMN_TYPE_MAP.put(Types.SMALLINT, MinorType.INT);
+ COLUMN_TYPE_MAP.put(Types.TINYINT, MinorType.INT);
+ COLUMN_TYPE_MAP.put(Types.DOUBLE, MinorType.FLOAT8);
+ COLUMN_TYPE_MAP.put(Types.FLOAT, MinorType.FLOAT8);
+ COLUMN_TYPE_MAP.put(Types.DECIMAL, MinorType.VARDECIMAL);
+ // time
+ COLUMN_TYPE_MAP.put(Types.DATE, MinorType.DATE);
+ COLUMN_TYPE_MAP.put(Types.TIME, MinorType.TIME);
+ COLUMN_TYPE_MAP.put(Types.TIMESTAMP, MinorType.TIMESTAMP);
+ // binary
+ COLUMN_TYPE_MAP.put(Types.BINARY, MinorType.VARBINARY); // Raw fixed length byte array. Mapped to byte[].
+ COLUMN_TYPE_MAP.put(Types.VARBINARY, MinorType.VARBINARY); // Raw variable length byte array.
+ // boolean
+ COLUMN_TYPE_MAP.put(Types.BOOLEAN, MinorType.BIT);
+ }
+
+ public abstract static class ColumnDefn {
+
+ final String name;
+ final int index;
+ final int sqlType;
+ ColumnWriter writer;
+
+ public String getName() {
+ return name;
+ }
+
+ public int getIndex() {
+ return index;
+ }
+
+ public int getSqlType() {
+ return sqlType;
+ }
+
+ public ColumnDefn(String name, int sqlType, int index) {
+ this.name = name;
+ this.sqlType = sqlType;
+ this.index = index;
+ }
+
+ public void define(SchemaBuilder builder) {
+ builder.addNullable(getName(), COLUMN_TYPE_MAP.get(getSqlType()));
+ }
+
+ public void bind(RowSetLoader loader) {
+ writer = loader.scalar(getName());
+ }
+
+ public abstract void load(Object value);
+ }
+
+ public static class GenericDefn extends ColumnDefn {
+
+ public GenericDefn(String name, int sqlType, int index) {
+ super(name, sqlType, index);
+ }
+
+ @Override
+ public void load(Object value) { // TODO refactor this to AbstractScalarWriter
+ ScalarWriter scalarWriter = (ScalarWriter) writer;
+ switch (getSqlType()) {
+ case Types.VARCHAR:
+ case Types.CHAR:
+ scalarWriter.setString((String) value);
+ break;
+ case Types.BIGINT :
+ scalarWriter.setLong((Long) value);
+ break;
+ case Types.INTEGER :
+ scalarWriter.setInt((Integer) value);
+ break;
+ case Types.SMALLINT :
+ scalarWriter.setInt((Short) value);
+ break;
+ case Types.TINYINT :
+ scalarWriter.setInt((Byte) value);
+ break;
+ case Types.DOUBLE :
+ case Types.FLOAT :
+ scalarWriter.setDouble((Double) value);
+ break;
+ case Types.DECIMAL :
+ scalarWriter.setDecimal((BigDecimal) value);
+ break;
+ case Types.DATE :
+ scalarWriter.setDate(((Date) value).toLocalDate());
+ break;
+ case Types.TIME :
+ scalarWriter.setTime(((Time) value).toLocalTime());
+ break;
+ case Types.TIMESTAMP :
+ scalarWriter.setTimestamp(((Timestamp) value).toInstant());
+ break;
+ case Types.BINARY :
+ case Types.VARBINARY :
+ byte[] byteValue = (byte[]) value;
+ scalarWriter.setBytes(byteValue, byteValue.length);
+ break;
+ case Types.BOOLEAN :
+ scalarWriter.setBoolean((Boolean) value);
+ break;
+ default:
+ break;
+ }
+ }
+ }
+
+ public static class ArrayDefn extends ColumnDefn {
+
+ final String VARCHAR = "VARCHAR ARRAY";
+ final String CHAR = "CHAR ARRAY";
+ final String BIGINT = "BIGINT ARRAY";
+ final String INTEGER = "INTEGER ARRAY";
+ final String DOUBLE = "DOUBLE ARRAY";
+ final String FLOAT = "FLOAT ARRAY";
+ final String SMALLINT = "SMALLINT ARRAY";
+ final String TINYINT = "TINYINT ARRAY";
+ final String BOOLEAN = "BOOLEAN ARRAY";
+
+ final String baseType;
+
+ public ArrayDefn(String name, int sqlType, String baseType, int index) {
+ super(name, sqlType, index);
+ this.baseType = baseType;
+ }
+
+ @Override
+ public void define(SchemaBuilder builder) {
+ switch (baseType) {
+ case VARCHAR:
+ case CHAR:
+ builder.addArray(getName(), MinorType.VARCHAR);
+ break;
+ case BIGINT:
+ builder.addArray(getName(), MinorType.BIGINT);
+ break;
+ case INTEGER:
+ builder.addArray(getName(), MinorType.INT);
+ break;
+ case DOUBLE:
+ case FLOAT:
+ builder.addArray(getName(), MinorType.FLOAT8);
+ break;
+ case SMALLINT:
+ builder.addArray(getName(), MinorType.SMALLINT);
+ break;
+ case TINYINT:
+ builder.addArray(getName(), MinorType.TINYINT);
+ break;
+ case BOOLEAN:
+ builder.addArray(getName(), MinorType.BIT);
+ break;
+ default:
+ break;
+ }
+ }
+
+ @Override
+ public void bind(RowSetLoader loader) {
+ writer = loader.array(getName());
+ }
+
+ @Override
+ public void load(Object value) {
Review comment:
As above: try to avoid a switch in the inner-most loop.
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixGroupScan.java
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.util.List;
+import java.util.Objects;
+
+import org.apache.drill.common.PlanStringBuilder;
+import org.apache.drill.common.exceptions.ExecutionSetupException;
+import org.apache.drill.common.expression.SchemaPath;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.exec.physical.PhysicalOperatorSetupException;
+import org.apache.drill.exec.physical.base.AbstractGroupScan;
+import org.apache.drill.exec.physical.base.GroupScan;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.physical.base.ScanStats;
+import org.apache.drill.exec.physical.base.ScanStats.GroupScanProperty;
+import org.apache.drill.exec.physical.base.SubScan;
+import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
+import org.apache.drill.exec.store.StoragePluginRegistry;
+
+import com.fasterxml.jackson.annotation.JacksonInject;
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+
+@JsonTypeName("phoenix-scan")
+public class PhoenixGroupScan extends AbstractGroupScan {
+
+ private final String sql;
+ private final List<SchemaPath> columns;
+ private final PhoenixScanSpec scanSpec;
+ private final double rows;
+ private final ScanStats scanStats;
+ private final PhoenixStoragePlugin plugin;
+
+ private int hashCode;
+
+ @JsonCreator
+ public PhoenixGroupScan(
+ @JsonProperty("sql") String sql,
+ @JsonProperty("columns") List<SchemaPath> columns,
+ @JsonProperty("scanSpec") PhoenixScanSpec scanSpec,
+ @JsonProperty("rows") double rows,
+ @JsonProperty("config") PhoenixStoragePluginConfig config,
+ @JacksonInject StoragePluginRegistry plugins) {
+ super("no-user");
+ this.sql = sql;
+ this.columns = columns;
+ this.scanSpec = scanSpec;
+ this.rows = rows;
+ this.scanStats = computeScanStats();
+ this.plugin = plugins.resolve(config, PhoenixStoragePlugin.class);
+ }
+
+ public PhoenixGroupScan(PhoenixScanSpec scanSpec, PhoenixStoragePlugin plugin) {
+ super("no-user");
+ this.sql = scanSpec.getSql();
+ this.columns = ALL_COLUMNS;
+ this.scanSpec = scanSpec;
+ this.rows = 100;
+ this.scanStats = computeScanStats();
+ this.plugin = plugin;
+ }
+
+ public PhoenixGroupScan(PhoenixGroupScan scan) {
+ super(scan);
+ this.sql = scan.sql;
+ this.columns = scan.columns;
+ this.scanSpec = scan.scanSpec;
+ this.rows = scan.rows;
+ this.scanStats = scan.scanStats;
+ this.plugin = scan.plugin;
+ }
+
+ public PhoenixGroupScan(PhoenixGroupScan scan, List<SchemaPath> columns) {
+ super(scan);
+ this.sql = scan.sql;
+ this.columns = columns;
+ this.scanSpec = scan.scanSpec;
+ this.rows = scan.rows;
+ this.scanStats = scan.scanStats;
+ this.plugin = scan.plugin;
+ }
+
+ public PhoenixGroupScan(String sql, List<SchemaPath> columns, PhoenixScanSpec scanSpec, double rows, PhoenixStoragePlugin plugin) {
+ super("no-user");
+ this.sql = sql;
+ this.columns = columns;
+ this.scanSpec = scanSpec;
+ this.rows = rows;
+ this.scanStats = computeScanStats();
+ this.plugin = plugin;
+ }
+
+ @JsonProperty("sql")
+ public String sql() {
+ return sql;
+ }
+
+ @JsonProperty("columns")
+ public List<SchemaPath> columns() {
+ return columns;
+ }
+
+ @JsonProperty("scanSpec")
+ public PhoenixScanSpec scanSpec() {
+ return scanSpec;
+ }
+
+ @JsonProperty("rows")
+ public double rows() {
+ return rows;
+ }
+
+ @JsonProperty("scanStats")
+ public ScanStats scanStats() {
+ return scanStats;
+ }
+
+ @JsonIgnore
+ public PhoenixStoragePlugin plugin() {
+ return plugin;
+ }
+
+ @JsonProperty("config")
+ public StoragePluginConfig config() {
+ return plugin.getConfig();
+ }
+
+ @Override
+ public void applyAssignments(List<DrillbitEndpoint> endpoints) throws PhysicalOperatorSetupException { }
+
+ @Override
+ public SubScan getSpecificScan(int minorFragmentId) throws ExecutionSetupException {
+ return new PhoenixSubScan(sql, columns, scanSpec, plugin);
+ }
+
+ @Override
+ public int getMaxParallelizationWidth() {
+ return 1;
+ }
+
+ @Override
+ public String getDigest() {
+ return toString();
+ }
+
+ @Override
+ public PhysicalOperator getNewWithChildren(List<PhysicalOperator> children) throws ExecutionSetupException {
+ return new PhoenixGroupScan(this);
+ }
+
+ @Override
+ public ScanStats getScanStats() {
+ return scanStats;
+ }
+
+ @Override
+ public GroupScan clone(List<SchemaPath> columns) {
+ return new PhoenixGroupScan(this, columns);
+ }
+
+ @Override
+ public int hashCode() {
+ if (hashCode == 0) {
+ hashCode = Objects.hash(sql, columns, scanSpec, rows, plugin.getConfig());
+ }
+ return hashCode;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if(this == obj) {
Review comment:
Nit: insert space after `if` here and below.
##########
File path: contrib/storage-phoenix/src/main/resources/bootstrap-storage-plugins.json
##########
@@ -0,0 +1,14 @@
+{
+ "storage": {
+ "phoenix": {
+ "type": "phoenix",
+ "jdbcURL": "jdbc:phoenix:thin:url=http://the.queryserver.hostname:8765;serialization=PROTOBUF",
+ "username": "drill",
+ "password": "drill",
Review comment:
Is this the default out-of-the-box Phoenix password? Probably not. Maybe we should use the default one so a simple install & run works without fiddling?
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/rules/PhoenixPrel.java
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.drill.exec.store.phoenix.rules;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+
+import org.apache.calcite.adapter.java.JavaTypeFactory;
+import org.apache.calcite.adapter.jdbc.JdbcImplementor;
+import org.apache.calcite.plan.ConventionTraitDef;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.AbstractRelNode;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelWriter;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.sql.SqlDialect;
+import org.apache.drill.common.expression.SchemaPath;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.planner.physical.PhysicalPlanCreator;
+import org.apache.drill.exec.planner.physical.Prel;
+import org.apache.drill.exec.planner.physical.visitor.PrelVisitor;
+import org.apache.drill.exec.record.BatchSchema.SelectionVectorMode;
+import org.apache.drill.exec.store.SubsetRemover;
+import org.apache.drill.exec.store.phoenix.PhoenixGroupScan;
+
+public class PhoenixPrel extends AbstractRelNode implements Prel {
+
+ private final String sql;
+ private final double rows;
+ private final PhoenixConvention convention;
+
+ public PhoenixPrel(RelOptCluster cluster, RelTraitSet traitSet, PhoenixIntermediatePrel prel) {
+ super(cluster, traitSet);
+ final RelNode input = prel.getInput();
+ rows = input.estimateRowCount(cluster.getMetadataQuery());
+ convention = (PhoenixConvention) input.getTraitSet().getTrait(ConventionTraitDef.INSTANCE);
+ final SqlDialect dialect = convention.getPlugin().getDialect();
+ final JdbcImplementor jdbcImplementor = new PhoenixImplementor(dialect, (JavaTypeFactory) getCluster().getTypeFactory());
+ final JdbcImplementor.Result result = jdbcImplementor.visitChild(0, input.accept(SubsetRemover.INSTANCE));
+ sql = result.asStatement().toSqlString(dialect).getSql();
+ rowType = input.getRowType();
+ }
+
+ @Override
+ public Iterator<Prel> iterator() {
+ return Collections.emptyIterator();
+ }
+
+ @Override
+ public PhysicalOperator getPhysicalOperator(PhysicalPlanCreator creator)
+ throws IOException {
+ List<SchemaPath> columns = new ArrayList<SchemaPath>();
+ for (String col : rowType.getFieldNames()) {
+ columns.add(SchemaPath.getSimplePath(col));
+ }
+ PhoenixGroupScan output = new PhoenixGroupScan(sql, columns, null, rows, convention.getPlugin());
+ return creator.addMetadata(this, output);
+ }
+
+ @Override
+ public RelWriter explainTerms(RelWriter pw) {
+ return super.explainTerms(pw).item("sql", stripToOneLineSql(sql));
+ }
+
+ private String stripToOneLineSql(String sql) {
+ StringBuilder sbt = new StringBuilder(sql.length());
+ String[] sqlToken = sql.split("\\n");
+ for (String sqlText : sqlToken) {
+ if (!sqlText.trim().startsWith("--")) {
Review comment:
Sadly, the following is also legal (in most SQL):
```
SELECT a, -- first comment
b, -- second comment
FROM ...
```
The above code will produce:
```sql
SELECT a, --first comment b -- second comment FROM ...
```
This leads to a question: why convert to a single line? Is this to convert SQL to Phoenix? Can't Phoenix parse newlines? (This would be very odd as the SQL standard says newlines are fine whitespace.)
If you do have to remove newlines, you may need to have a mini-parser to handle comments and newlines.
[This page](https://phoenix.apache.org/language/index.html#comments) suggests that Phoenix can handle comments and newlines.
##########
File path: contrib/storage-phoenix/src/main/java/org/apache/drill/exec/store/phoenix/PhoenixBatchReader.java
##########
@@ -0,0 +1,370 @@
+/*
+ * 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.drill.exec.store.phoenix;
+
+import java.math.BigDecimal;
+import java.sql.Array;
+import java.sql.Connection;
+import java.sql.Date;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Time;
+import java.sql.Timestamp;
+import java.sql.Types;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.drill.common.AutoCloseables;
+import org.apache.drill.common.exceptions.CustomErrorContext;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.expression.SchemaPath;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.physical.impl.scan.framework.ManagedReader;
+import org.apache.drill.exec.physical.impl.scan.framework.SchemaNegotiator;
+import org.apache.drill.exec.physical.resultSet.RowSetLoader;
+import org.apache.drill.exec.record.metadata.SchemaBuilder;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+import org.apache.drill.exec.vector.accessor.ColumnWriter;
+import org.apache.drill.exec.vector.accessor.ScalarWriter;
+import org.apache.drill.shaded.guava.com.google.common.base.Stopwatch;
+import org.apache.drill.shaded.guava.com.google.common.collect.Maps;
+import org.slf4j.LoggerFactory;
+
+public class PhoenixBatchReader implements ManagedReader<SchemaNegotiator> {
+
+ private static final org.slf4j.Logger logger = LoggerFactory.getLogger(PhoenixBatchReader.class);
+
+ private final PhoenixSubScan subScan;
+ private CustomErrorContext errorContext;
+ private PhoenixReader reader;
+ private Connection conn;
+ private PreparedStatement pstmt;
+ private ResultSet rs;
+ private ResultSetMetaData meta;
+ private ColumnDefn[] columns;
+ private Stopwatch watch;
+ private int count = 0;
+
+ public PhoenixBatchReader(PhoenixSubScan subScan) {
+ this.subScan = subScan;
+ }
+
+ @Override
+ public boolean open(SchemaNegotiator negotiator) {
+ try {
+ errorContext = negotiator.parentErrorContext();
+ conn = subScan.getPlugin().getDataSource().getConnection();
+ pstmt = conn.prepareStatement(subScan.getSql());
+ rs = pstmt.executeQuery();
+ meta = pstmt.getMetaData();
+ } catch (SQLException e) {
+ throw UserException
+ .dataReadError(e)
+ .message("Failed to execute the phoenix sql query. " + e.getMessage())
+ .build(logger);
+ }
+ try {
+ negotiator.tableSchema(defineMetadata(), true);
+ reader = new PhoenixReader(negotiator.build());
+ bindColumns(reader.getStorage());
+ } catch (SQLException e) {
+ throw UserException
+ .dataReadError(e)
+ .message("Failed to get type of columns from metadata. " + e.getMessage())
+ .build(logger);
+ }
+ watch = Stopwatch.createStarted();
+ return true;
+ }
+
+ @Override
+ public boolean next() {
+ try {
+ while (rs.next()) {
+ { // TODO refactor this to PhoenixReader
+ reader.getStorage().start();
+ for (int index = 0; index < columns.length; index++) {
+ if (columns[index].getSqlType() == Types.ARRAY) {
+ Array result = rs.getArray(index + 1);
+ if (result != null) {
+ columns[index].load(result.getArray());
+ }
+ } else {
+ columns[index].load(rs.getObject(index + 1));
+ }
+ }
+ count++;
+ reader.getStorage().save();
+ }
+ if (reader.getStorage().isFull()) { // batch full but not reached the EOF
+ return true;
+ }
+ }
+ } catch (SQLException e) {
+ throw UserException
+ .dataReadError(e)
+ .message("Failed to get the data from the result set. " + e.getMessage())
+ .build(logger);
+ }
+ watch.stop();
+ logger.debug("Phoenix fetch total record numbers : {}", count);
+ return false; // the EOF is reached.
+ }
+
+ @Override
+ public void close() {
+ count = reader.getStorage().loader().batchCount();
+ logger.debug("Phoenix fetch batch size : {}, took {} ms. ", count, watch.elapsed(TimeUnit.MILLISECONDS));
+ AutoCloseables.closeSilently(rs, pstmt, conn);
+ }
+
+ private TupleMetadata defineMetadata() throws SQLException {
+ List<SchemaPath> cols = subScan.getColumns();
+ columns = new ColumnDefn[cols.size()];
+ SchemaBuilder builder = new SchemaBuilder();
+ for (int index = 0; index < cols.size(); index++) {
+ int sqlType = meta.getColumnType(index + 1); // column the first column is 1
+ String columnName = cols.get(index).rootName();
+ columns[index] = makeColumn(columnName, sqlType, meta.getColumnTypeName(index + 1), index);
+ columns[index].define(builder);
+ }
+ return builder.buildSchema();
+ }
+
+ private ColumnDefn makeColumn(String name, int sqlType, String baseType, int index) {
+ if (sqlType == Types.ARRAY) {
+ return new ArrayDefn(name, sqlType, baseType, index);
+ }
+ return new GenericDefn(name, sqlType, index);
+ }
+
+ private void bindColumns(RowSetLoader loader) {
+ for (int i = 0; i < columns.length; i++) {
+ columns[i].bind(loader);
+ }
+ }
+
+ protected static final Map<Integer, MinorType> COLUMN_TYPE_MAP = Maps.newHashMap();
+
+ static {
+ // text
+ COLUMN_TYPE_MAP.put(Types.VARCHAR, MinorType.VARCHAR);
+ COLUMN_TYPE_MAP.put(Types.CHAR, MinorType.VARCHAR);
+ // numbers
+ COLUMN_TYPE_MAP.put(Types.BIGINT, MinorType.BIGINT);
+ COLUMN_TYPE_MAP.put(Types.INTEGER, MinorType.INT);
+ COLUMN_TYPE_MAP.put(Types.SMALLINT, MinorType.INT);
+ COLUMN_TYPE_MAP.put(Types.TINYINT, MinorType.INT);
+ COLUMN_TYPE_MAP.put(Types.DOUBLE, MinorType.FLOAT8);
+ COLUMN_TYPE_MAP.put(Types.FLOAT, MinorType.FLOAT8);
+ COLUMN_TYPE_MAP.put(Types.DECIMAL, MinorType.VARDECIMAL);
+ // time
+ COLUMN_TYPE_MAP.put(Types.DATE, MinorType.DATE);
+ COLUMN_TYPE_MAP.put(Types.TIME, MinorType.TIME);
+ COLUMN_TYPE_MAP.put(Types.TIMESTAMP, MinorType.TIMESTAMP);
+ // binary
+ COLUMN_TYPE_MAP.put(Types.BINARY, MinorType.VARBINARY); // Raw fixed length byte array. Mapped to byte[].
+ COLUMN_TYPE_MAP.put(Types.VARBINARY, MinorType.VARBINARY); // Raw variable length byte array.
+ // boolean
+ COLUMN_TYPE_MAP.put(Types.BOOLEAN, MinorType.BIT);
+ }
+
+ public abstract static class ColumnDefn {
+
+ final String name;
+ final int index;
+ final int sqlType;
+ ColumnWriter writer;
+
+ public String getName() {
+ return name;
+ }
+
+ public int getIndex() {
+ return index;
+ }
+
+ public int getSqlType() {
+ return sqlType;
+ }
+
+ public ColumnDefn(String name, int sqlType, int index) {
+ this.name = name;
+ this.sqlType = sqlType;
+ this.index = index;
+ }
+
+ public void define(SchemaBuilder builder) {
+ builder.addNullable(getName(), COLUMN_TYPE_MAP.get(getSqlType()));
+ }
+
+ public void bind(RowSetLoader loader) {
+ writer = loader.scalar(getName());
+ }
+
+ public abstract void load(Object value);
+ }
+
+ public static class GenericDefn extends ColumnDefn {
+
+ public GenericDefn(String name, int sqlType, int index) {
+ super(name, sqlType, index);
+ }
+
+ @Override
+ public void load(Object value) { // TODO refactor this to AbstractScalarWriter
+ ScalarWriter scalarWriter = (ScalarWriter) writer;
+ switch (getSqlType()) {
Review comment:
Suggestion: we're in the innermost loop here: reading every column for every row. The preferred way to handle multiple types is with a class (or lambda) that we can jump to directly without the indirection of a switch. In existing code, we tend to create a class per type. Might be fun to try the more modern approach a lambda that implements `Consumer`. The result is that the virtual function call to `load()` gets you directly to the code that sets the value: no need for a per-column switch statement.
##########
File path: exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/writer/ScalarArrayWriter.java
##########
@@ -174,6 +174,14 @@ public void setObject(Object array) {
setLongObjectArray((Long[]) array);
} else if (memberClassName.equals(Double.class.getName())) {
setDoubleObjectArray((Double[]) array);
+ } else if (memberClassName.equals(Float.class.getName())) {
+ setFloatObjectArray((Float[]) array);
+ } else if (memberClassName.equals(Short.class.getName())) {
+ setShortObjectArray((Short[]) array);
+ } else if (memberClassName.equals(Byte.class.getName())) {
+ setByteObjectArray((Byte[]) array);
+ } else if (memberClassName.equals(Boolean.class.getName())) {
+ setBooleanObjectArray((Boolean[]) array);
Review comment:
Thanks for adding these missing types! Please add unit tests for these changes. I believe there are tests which exercise this code; you can just add new cases for the new types.
##########
File path: exec/vector/src/main/java/org/apache/drill/exec/vector/accessor/writer/ScalarArrayWriter.java
##########
@@ -201,12 +220,34 @@ public void setByteArray(byte[] value) {
}
}
+ public void setByteObjectArray(Byte[] value) {
+ for (int i = 0; i < value.length; i++) {
+ final Byte element = value[i];
+ if (element == null) {
+ elementWriter.setNull();
+ } else {
+ elementWriter.setInt(element);
+ }
+ }
+ }
+
public void setShortArray(short[] value) {
for (int i = 0; i < value.length; i++) {
elementWriter.setInt(value[i]);
}
}
+ public void setShortObjectArray(Short[] value) {
+ for (int i = 0; i < value.length; i++) {
+ final Short element = value[i];
+ if (element == null) {
+ elementWriter.setNull();
Review comment:
Was this tested? As it turns out, Drill does not allow individual array elements to be null, so I believe this call will throw an exception. Drill only allows the entire array to be empty (which Drill treats equivalent to NULL, IIRC.)
One way to handle nulls is to replace them with a default value. Since that default value is likely to be datasource-specific, the conversion should be done by the caller.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Z0ltrix commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
Z0ltrix commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-959753259
> @cgivre Thanks for the information. Actually, I have already tried to use the PQS test framework into Drill. Unfortunately, the Hadoop test dependencies (2.7) and HBase test dependencies (1.4) caused a lot of conflict (Drill use Hadoop 3.2 and HBase 2.4). That's one of the reasons I chose to make a container image.
Hi @luocooong ,
as you can see at the project downloads page, phoenix is shipped in several version-combinations.
For the newest phoenix version > 5 its only build against hbase 2.x and hadoop 3.x
https://github.com/apache/phoenix/blob/master/pom.xml#L78
In fact that hbase 2.3.7 is the current stable release and it recommends hadoop 2.10.0 or 3.1.2, and Drill is already on hadoop 3, i would go with that constellation.
https://github.com/apache/hbase/blob/rel/2.3.7/pom.xml#L1349
phoenix == 5.1.2
PQS == 6.0.0
hbase == 2.3.7
hadoop >= 3.1.2
Hope this thoughts help
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-958973388
@luocooong Thanks for this PR. This will be a really great addition to Drill! I wanted to suggest that since there isn't a testcontainer for Apache Phoenix, that it might be a better approach to use the Phoenix test classes. Here's one that I found: https://github.com/apache/phoenix/blob/master/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
Maybe take a look and see what you think. From my recollection, this is what we had to do with the Splunk connector.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Z0ltrix commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
Z0ltrix commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-959753259
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-958973388
@luocooong Thanks for this PR. This will be a really great addition to Drill! I wanted to suggest that since there isn't a testcontainer for Apache Phoenix, that it might be a better approach to use the Phoenix test classes. Here's one that I found: https://github.com/apache/phoenix/blob/master/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
Maybe take a look and see what you think. From my recollection, this is what we had to do with the Splunk connector.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-958973388
@luocooong Thanks for this PR. This will be a really great addition to Drill! I wanted to suggest that since there isn't a testcontainer for Apache Phoenix, that it might be a better approach to use the Phoenix test classes. Here's one that I found: https://github.com/apache/phoenix/blob/master/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
Maybe take a look and see what you think. From my recollection, this is what we had to do with the Splunk connector.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] dzamo edited a comment on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
dzamo edited a comment on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-960890029
> Hi @dzamo , phoenix uses keytab and other stuff within the connection string: https://phoenix.apache.org/#connStr isnt this what we need?
>
> Or, the other aspect... use code from PQS for the connection to phoenix... they use some sort of proxyUser from org.apache.hadoop.security.UserGroupInformation as well: https://github.com/apache/phoenix-queryserver/blob/master/phoenix-queryserver/src/main/java/org/apache/phoenix/queryserver/server/QueryServer.java#L553
>
> Regards, Christian
@Z0ltrix Okaaay. I'd forgotten that Phoenix lives on top of Hadoop anyway. So if you've already enabled Hadoop's impersonation using the needed `proxyuser` settings then maybe all that we need here is to tack a `doAs` onto the JDBC URL you already have working (which might already include some Kerberos bits).
```
jdbc:phoenix:thin:url=http://localhost:8765?doAs=alice
```
See https://phoenix.apache.org/server.html#Impersonation. @luocooong this should be reasonably easy, let me know if I can help with anything. We should imitate the behaviour of storage-hive which also contains "doAs". In my head it's something like this farcically simplified pseudocode:
```
if (config.getOption('drill.exec.impersonation.enabled') == true) {
phoenixJdbcUrl = phoenixJdbcUrl + "?doAs=" + activeDrillUser;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on a change in pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
luocooong commented on a change in pull request #2332:
URL: https://github.com/apache/drill/pull/2332#discussion_r760832203
##########
File path: contrib/storage-phoenix/README.md
##########
@@ -0,0 +1,98 @@
+# [DRILL-7863](https://issues.apache.org/jira/browse/DRILL-7863): Add Storage Plugin for Apache Phoenix
+
+## Description
+
+ Phoenix say : "We put the SQL back in NoSQL",<br/>
+ Drill call : "We use the SQL to cross almost all the file systems and storage engines",<br/>
+ "Cheers !", users said.
+
+## Documentation
+
+Features :
+
+ - Full support for Enhanced Vector Framework.
+
+ - Tested in phoenix 4.14 and 5.1.2.
+
+ - Support the array data type.
+
+ - Support the pushdown (Project, Limit, Filter, Aggregate, Join, CrossJoin, Join_Filter, GroupBy, Distinct and more).
+
+ - Use the PQS client (6.0).
+
+Related Information :
+
+ 1. PHOENIX-6398: Returns uniform SQL dialect in calcite for the PQS
+
+ 2. PHOENIX-6582: Bump default HBase version to 2.3.7 and 2.4.8
+
+ 3. PHOENIX-6605, PHOENIX-6606 and PHOENIX-6607.
+
+ 4. DRILL-8060, DRILL-8061 and DRILL-8062.
+
+ 5. [QueryServer 6.0.0-drill-r1](https://github.com/luocooong/phoenix-queryserver/releases/tag/6.0.0-drill-r1)
+
+## Testing
+
+ The test framework of phoenix queryserver required the Hadoop 3, but exist `PHOENIX-5993` and `HBASE-22394` :
+
+```
+" The HBase PMC does not release multiple artifacts for both Hadoop2 and Hadoop3 support at the current time.
+Current HBase2 releases still compile against Hadoop2 by default, and using Hadoop 3 against HBase2
+requires a recompilation of HBase because of incompatible changes between Hadoop2 and Hadoop3. "
+```
+
+### Recommended Practices
+
+ 1. Download HBase 2.4.2 sources and rebuild with Hadoop 3.
+
+ 2. Remove the `Ignore` annotation in `PhoenixTestSuite.java`.
+
+ 3. Go to the phoenix root folder and run test.
+
+### To Add Features
+
+ - Don't forget to add a test function to the test class.
+
+ - If a new test class is added, please declare it in the `PhoenixTestSuite` class.
+
+### Play in CLI
+
+```sql
Review comment:
@paul-rogers Thank you very much. Does it mean that the "sql" character can be removed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] luocooong commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
luocooong commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-981112511
@paul-rogers Thank you very much!
I took all your suggestions, except the default port. Because after I tried it, found that if the port is not specified (in the URL), the driver will use Avatica 80 port instead of PQS 8765 port (default).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] Z0ltrix commented on pull request #2332: DRILL-7863: Add Storage Plugin for Apache Phoenix
Posted by GitBox <gi...@apache.org>.
Z0ltrix commented on pull request #2332:
URL: https://github.com/apache/drill/pull/2332#issuecomment-959753259
> @cgivre Thanks for the information. Actually, I have already tried to use the PQS test framework into Drill. Unfortunately, the Hadoop test dependencies (2.7) and HBase test dependencies (1.4) caused a lot of conflict (Drill use Hadoop 3.2 and HBase 2.4). That's one of the reasons I chose to make a container image.
Hi @luocooong ,
as you can see at the project downloads page, phoenix is shipped in several version-combinations.
For the newest phoenix version > 5 its only build against hbase 2.x and hadoop 3.x
https://github.com/apache/phoenix/blob/master/pom.xml#L78
In fact that hbase 2.3.7 is the current stable release and it recommends hadoop 2.10.0 or 3.1.2, and Drill is already on hadoop 3, i would go with that constellation.
https://github.com/apache/hbase/blob/rel/2.3.7/pom.xml#L1349
phoenix == 5.1.2
PQS == 6.0.0
hbase == 2.3.7
hadoop >= 3.1.2
Hope this thoughts help
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org