Skip to content

Commit

Permalink
DRILL-6588: Make Sys tables of nullable datatypes
Browse files Browse the repository at this point in the history
This is to address the generic problem of NULL values being projected as a string because of all datatypes being non-nullable.
This patch only applies to tables backed by the PojoReader (in our case, System tables). Added NonNullable annotations wherever application in any of the System tables, along with a unit test that verifies both nullable and non-nullable datatypes exist in the system tables.

closes #1371
  • Loading branch information
kkhatua authored and sohami committed Jul 14, 2018
1 parent e3534b6 commit bd6f63d
Show file tree
Hide file tree
Showing 12 changed files with 116 additions and 36 deletions.
Expand Up @@ -17,22 +17,23 @@
*/ */
package org.apache.drill.exec.store; package org.apache.drill.exec.store;


import com.google.common.collect.Lists; import java.util.AbstractMap.SimpleImmutableEntry;
import java.util.ArrayList;
import java.util.List;

import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory; import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.sql.type.SqlTypeName; import org.apache.calcite.sql.type.SqlTypeName;


import java.util.List;

/** /**
* RecordDataType defines names and data types of columns in a static drill table. * RecordDataType defines names and data types of columns in a static drill table.
*/ */
public abstract class RecordDataType { public abstract class RecordDataType {


/** /**
* @return the {@link org.apache.calcite.sql.type.SqlTypeName} of columns in the table * @return the {@link org.apache.calcite.sql.type.SqlTypeName} of columns in the table as a pair with its nullability
*/ */
public abstract List<SqlTypeName> getFieldSqlTypeNames(); public abstract List<SimpleImmutableEntry<SqlTypeName, Boolean>> getFieldSqlTypeNames();


/** /**
* @return the column names in the table * @return the column names in the table
Expand All @@ -47,17 +48,21 @@ public abstract class RecordDataType {
* @return the constructed type * @return the constructed type
*/ */
public final RelDataType getRowType(RelDataTypeFactory factory) { public final RelDataType getRowType(RelDataTypeFactory factory) {
final List<SqlTypeName> types = getFieldSqlTypeNames(); final List<SimpleImmutableEntry<SqlTypeName, Boolean>> types = getFieldSqlTypeNames();
final List<String> names = getFieldNames(); final List<String> names = getFieldNames();
final List<RelDataType> fields = Lists.newArrayList(); final List<RelDataType> fields = new ArrayList<>();
for (final SqlTypeName typeName : types) { for (SimpleImmutableEntry<SqlTypeName, Boolean> sqlTypePair : types) {
final SqlTypeName typeName = sqlTypePair.getKey();
final RelDataType tempDataType;
switch (typeName) { switch (typeName) {
case VARCHAR: case VARCHAR:
fields.add(factory.createSqlType(typeName, Integer.MAX_VALUE)); tempDataType = factory.createSqlType(typeName, Integer.MAX_VALUE);
break; break;
default: default:
fields.add(factory.createSqlType(typeName)); tempDataType = factory.createSqlType(typeName);
} }
//Add [Non]Nullable RelDataType
fields.add(factory.createTypeWithNullability(tempDataType, sqlTypePair.getValue()));
} }
return factory.createStructType(fields, names); return factory.createStructType(fields, names);
} }
Expand Down
@@ -0,0 +1,32 @@
/*
* 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.pojo;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* Indicates NonNullable nature
*/
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.FIELD)
public @interface NonNullable {
boolean value() default true;
}
Expand Up @@ -21,11 +21,11 @@
import java.lang.reflect.Modifier; import java.lang.reflect.Modifier;
import java.math.BigDecimal; import java.math.BigDecimal;
import java.sql.Timestamp; import java.sql.Timestamp;
import java.util.AbstractMap.SimpleImmutableEntry;
import java.util.ArrayList;
import java.util.List; import java.util.List;


import org.apache.calcite.sql.type.SqlTypeName; import org.apache.calcite.sql.type.SqlTypeName;

import com.google.common.collect.Lists;
import org.apache.drill.exec.store.RecordDataType; import org.apache.drill.exec.store.RecordDataType;


/** /**
Expand All @@ -34,8 +34,8 @@
public class PojoDataType extends RecordDataType { public class PojoDataType extends RecordDataType {
// private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(PojoDataType.class); // private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(PojoDataType.class);


private final List<SqlTypeName> types = Lists.newArrayList(); private final List<SimpleImmutableEntry<SqlTypeName, Boolean>> types = new ArrayList<>();
private final List<String> names = Lists.newArrayList(); private final List<String> names = new ArrayList<>();
private final Class<?> pojoClass; private final Class<?> pojoClass;


public PojoDataType(Class<?> pojoClass) { public PojoDataType(Class<?> pojoClass) {
Expand All @@ -48,22 +48,25 @@ public PojoDataType(Class<?> pojoClass) {
Class<?> type = f.getType(); Class<?> type = f.getType();
names.add(f.getName()); names.add(f.getName());


//Absence of annotation @NonNullable => (isNullable=true)
final boolean isNullable = !(f.isAnnotationPresent(NonNullable.class));

if (type == int.class || type == Integer.class) { if (type == int.class || type == Integer.class) {
types.add(SqlTypeName.INTEGER); types.add(new SimpleImmutableEntry<SqlTypeName, Boolean>(SqlTypeName.INTEGER, isNullable));
} else if(type == boolean.class || type == Boolean.class) { } else if(type == boolean.class || type == Boolean.class) {
types.add(SqlTypeName.BOOLEAN); types.add(new SimpleImmutableEntry<SqlTypeName, Boolean>(SqlTypeName.BOOLEAN, isNullable));
} else if(type == long.class || type == Long.class) { } else if(type == long.class || type == Long.class) {
types.add(SqlTypeName.BIGINT); types.add(new SimpleImmutableEntry<SqlTypeName, Boolean>(SqlTypeName.BIGINT, isNullable));
} else if(type == double.class || type == Double.class) { } else if(type == double.class || type == Double.class) {
types.add(SqlTypeName.DOUBLE); types.add(new SimpleImmutableEntry<SqlTypeName, Boolean>(SqlTypeName.DOUBLE, isNullable));
} else if(type == BigDecimal.class) { } else if(type == BigDecimal.class) {
types.add(SqlTypeName.DECIMAL); types.add(new SimpleImmutableEntry<SqlTypeName, Boolean>(SqlTypeName.DECIMAL, isNullable));
} else if(type == String.class) { } else if(type == String.class) {
types.add(SqlTypeName.VARCHAR); types.add(new SimpleImmutableEntry<SqlTypeName, Boolean>(SqlTypeName.VARCHAR, isNullable));
} else if(type.isEnum()) { } else if(type.isEnum()) {
types.add(SqlTypeName.VARCHAR); types.add(new SimpleImmutableEntry<SqlTypeName, Boolean>(SqlTypeName.VARCHAR, isNullable));
} else if (type == Timestamp.class) { } else if (type == Timestamp.class) {
types.add(SqlTypeName.TIMESTAMP); types.add(new SimpleImmutableEntry<SqlTypeName, Boolean>(SqlTypeName.TIMESTAMP, isNullable));
} else { } else {
throw new RuntimeException(String.format("PojoDataType doesn't yet support conversions from type [%s].", type)); throw new RuntimeException(String.format("PojoDataType doesn't yet support conversions from type [%s].", type));
} }
Expand All @@ -75,13 +78,12 @@ public Class<?> getPojoClass() {
} }


@Override @Override
public List<SqlTypeName> getFieldSqlTypeNames() { public List<SimpleImmutableEntry<SqlTypeName, Boolean>> getFieldSqlTypeNames() {
return types; return types;
} }


@Override @Override
public List<String> getFieldNames() { public List<String> getFieldNames() {
return names; return names;
} }

} }
Expand Up @@ -21,8 +21,8 @@
import java.util.Iterator; import java.util.Iterator;
import java.util.LinkedList; import java.util.LinkedList;
import java.util.List; import java.util.List;
import java.util.Set;
import java.util.Map.Entry; import java.util.Map.Entry;
import java.util.Set;
import java.util.TimeZone; import java.util.TimeZone;


import org.apache.drill.exec.ExecConstants; import org.apache.drill.exec.ExecConstants;
Expand All @@ -32,6 +32,7 @@
import org.apache.drill.exec.rpc.user.UserSession; import org.apache.drill.exec.rpc.user.UserSession;
import org.apache.drill.exec.server.options.OptionManager; import org.apache.drill.exec.server.options.OptionManager;
import org.apache.drill.exec.server.rest.profile.SimpleDurationFormat; import org.apache.drill.exec.server.rest.profile.SimpleDurationFormat;
import org.apache.drill.exec.store.pojo.NonNullable;
import org.apache.drill.exec.util.ImpersonationUtil; import org.apache.drill.exec.util.ImpersonationUtil;
import org.joda.time.DateTime; import org.joda.time.DateTime;


Expand Down Expand Up @@ -99,14 +100,18 @@ public void remove() {


public static class ConnectionInfo { public static class ConnectionInfo {
public String user; public String user;
@NonNullable
public String client; public String client;
@NonNullable
public String drillbit; public String drillbit;
@NonNullable
public Timestamp established; public Timestamp established;
public String duration; public String duration;
public int queries; public int queries;
public boolean isAuthenticated; public boolean isAuthenticated;
public boolean isEncrypted; public boolean isEncrypted;
public boolean usingSSL; public boolean usingSSL;
@NonNullable
public String session; public String session;


public ConnectionInfo(Entry<BitToUserConnection, BitToUserConnectionConfig> connectionConfigPair, String hostname) { public ConnectionInfo(Entry<BitToUserConnection, BitToUserConnectionConfig> connectionConfigPair, String hostname) {
Expand Down
Expand Up @@ -21,6 +21,7 @@


import org.apache.drill.exec.ops.ExecutorFragmentContext; import org.apache.drill.exec.ops.ExecutorFragmentContext;
import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint; import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
import org.apache.drill.exec.store.pojo.NonNullable;


public class DrillbitIterator implements Iterator<Object> { public class DrillbitIterator implements Iterator<Object> {
static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillbitIterator.class); static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillbitIterator.class);
Expand All @@ -34,13 +35,17 @@ public DrillbitIterator(ExecutorFragmentContext c) {
} }


public static class DrillbitInstance { public static class DrillbitInstance {
@NonNullable
public String hostname; public String hostname;
public int user_port; public int user_port;
public int control_port; public int control_port;
public int data_port; public int data_port;
public int http_port; public int http_port;
@NonNullable
public boolean current; public boolean current;
@NonNullable
public String version; public String version;
@NonNullable
public String state; public String state;
} }


Expand Down
Expand Up @@ -24,13 +24,14 @@
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;



import com.google.common.collect.Lists;
import org.apache.drill.exec.ops.FragmentContext; import org.apache.drill.exec.ops.FragmentContext;
import org.apache.drill.exec.server.options.OptionManager; import org.apache.drill.exec.server.options.OptionManager;
import org.apache.drill.exec.server.options.OptionValue; import org.apache.drill.exec.server.options.OptionValue;
import org.apache.drill.exec.server.options.OptionValue.Kind; import org.apache.drill.exec.server.options.OptionValue.Kind;
import org.apache.drill.exec.server.options.OptionValue.OptionScope; import org.apache.drill.exec.server.options.OptionValue.OptionScope;
import org.apache.drill.exec.store.pojo.NonNullable;

import com.google.common.collect.Lists;


/* /*
* Extends the original Option iterator. The idea is to hide the implementation details and present the * Extends the original Option iterator. The idea is to hide the implementation details and present the
Expand Down Expand Up @@ -134,10 +135,14 @@ public enum Status {
*/ */
public static class ExtendedOptionValueWrapper { public static class ExtendedOptionValueWrapper {


@NonNullable
public final String name; public final String name;
@NonNullable
public final String kind; public final String kind;
@NonNullable
public final OptionValue.AccessibleScopes accessibleScopes; public final OptionValue.AccessibleScopes accessibleScopes;
public final String val; public final String val;
@NonNullable
public final OptionScope optionScope; public final OptionScope optionScope;




Expand Down
Expand Up @@ -27,6 +27,7 @@
import org.apache.drill.common.config.DrillConfig; import org.apache.drill.common.config.DrillConfig;
import org.apache.drill.exec.ops.ExecutorFragmentContext; import org.apache.drill.exec.ops.ExecutorFragmentContext;
import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint; import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
import org.apache.drill.exec.store.pojo.NonNullable;


public class MemoryIterator implements Iterator<Object> { public class MemoryIterator implements Iterator<Object> {


Expand Down Expand Up @@ -83,6 +84,7 @@ public void remove() {
} }


public static class MemoryInfo { public static class MemoryInfo {
@NonNullable
public String hostname; public String hostname;
public long user_port; public long user_port;
public long heap_current; public long heap_current;
Expand Down
Expand Up @@ -21,15 +21,17 @@
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;


import com.google.common.collect.Iterators;
import com.google.common.collect.Lists;
import org.apache.drill.exec.ops.FragmentContext; import org.apache.drill.exec.ops.FragmentContext;
import org.apache.drill.exec.server.options.DrillConfigIterator; import org.apache.drill.exec.server.options.DrillConfigIterator;
import org.apache.drill.exec.server.options.OptionManager; import org.apache.drill.exec.server.options.OptionManager;
import org.apache.drill.exec.server.options.OptionValue; import org.apache.drill.exec.server.options.OptionValue;
import org.apache.drill.exec.server.options.OptionValue.AccessibleScopes;
import org.apache.drill.exec.server.options.OptionValue.Kind; import org.apache.drill.exec.server.options.OptionValue.Kind;
import org.apache.drill.exec.server.options.OptionValue.OptionScope; import org.apache.drill.exec.server.options.OptionValue.OptionScope;
import org.apache.drill.exec.server.options.OptionValue.AccessibleScopes; import org.apache.drill.exec.store.pojo.NonNullable;

import com.google.common.collect.Iterators;
import com.google.common.collect.Lists;


public class OptionIterator implements Iterator<Object> { public class OptionIterator implements Iterator<Object> {
// private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(OptionIterator.class); // private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(OptionIterator.class);
Expand Down Expand Up @@ -104,10 +106,15 @@ public enum Status {
*/ */
public static class OptionValueWrapper { public static class OptionValueWrapper {


@NonNullable
public final String name; public final String name;
@NonNullable
public final Kind kind; public final Kind kind;
@NonNullable
public final AccessibleScopes accessibleScopes; public final AccessibleScopes accessibleScopes;
@NonNullable
public final OptionScope optionScope; public final OptionScope optionScope;
@NonNullable
public final Status status; public final Status status;
public final Long num_val; public final Long num_val;
public final String string_val; public final String string_val;
Expand Down
Expand Up @@ -17,17 +17,19 @@
*/ */
package org.apache.drill.exec.store.sys; package org.apache.drill.exec.store.sys;


import com.google.common.base.Function; import java.sql.Timestamp;
import com.google.common.collect.Iterators; import java.util.Iterator;
import java.util.Map.Entry;

import javax.annotation.Nullable;


import org.apache.drill.exec.ops.ExecutorFragmentContext; import org.apache.drill.exec.ops.ExecutorFragmentContext;
import org.apache.drill.exec.proto.UserBitShared; import org.apache.drill.exec.proto.UserBitShared;
import org.apache.drill.exec.proto.UserBitShared.QueryProfile; import org.apache.drill.exec.proto.UserBitShared.QueryProfile;
import org.apache.drill.exec.store.pojo.NonNullable;


import javax.annotation.Nullable; import com.google.common.base.Function;
import java.sql.Timestamp; import com.google.common.collect.Iterators;
import java.util.Iterator;
import java.util.Map.Entry;


/** /**
* System table listing completed profiles * System table listing completed profiles
Expand Down Expand Up @@ -114,8 +116,10 @@ public static class ProfileInfo {


private static final ProfileInfo DEFAULT = new ProfileInfo(); private static final ProfileInfo DEFAULT = new ProfileInfo();


@NonNullable
public final String queryId; public final String queryId;
public final Timestamp startTime; public final Timestamp startTime;
@NonNullable
public final String foreman; public final String foreman;
public final long fragments; public final long fragments;
public final String user; public final String user;
Expand Down
Expand Up @@ -27,6 +27,7 @@
import org.apache.drill.exec.proto.UserBitShared; import org.apache.drill.exec.proto.UserBitShared;
import org.apache.drill.exec.proto.UserBitShared.QueryProfile; import org.apache.drill.exec.proto.UserBitShared.QueryProfile;
import org.apache.drill.exec.serialization.InstanceSerializer; import org.apache.drill.exec.serialization.InstanceSerializer;
import org.apache.drill.exec.store.pojo.NonNullable;


import com.google.common.base.Function; import com.google.common.base.Function;
import com.google.common.collect.Iterators; import com.google.common.collect.Iterators;
Expand Down Expand Up @@ -114,6 +115,7 @@ public static class ProfileJson {


private static final ProfileJson DEFAULT = new ProfileJson(); private static final ProfileJson DEFAULT = new ProfileJson();


@NonNullable
public final String queryId; public final String queryId;
public final String json; public final String json;


Expand Down
Expand Up @@ -23,6 +23,7 @@


import org.apache.drill.exec.ops.ExecutorFragmentContext; import org.apache.drill.exec.ops.ExecutorFragmentContext;
import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint; import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
import org.apache.drill.exec.store.pojo.NonNullable;


public class ThreadsIterator implements Iterator<Object> { public class ThreadsIterator implements Iterator<Object> {


Expand Down Expand Up @@ -62,6 +63,7 @@ public void remove() {
} }


public static class ThreadsInfo { public static class ThreadsInfo {
@NonNullable
public String hostname; public String hostname;
public long user_port; public long user_port;
public long total_threads; public long total_threads;
Expand Down

0 comments on commit bd6f63d

Please sign in to comment.