[GitHub] drill pull request: Bugs/various work re master 07 09
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/87 Bugs/various work re master 07 09 You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/various_WORK_re_master_07-09 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/87.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #87 commit 92e2cc18c79dbf5269304d1479f1b6ff8bcd477e Author: dbarclay Date: 2015-03-23T21:16:48Z sep.: --- forkCount 2 -> 1 [pom.xml] --- >= Working, < Review commit 59608f3664cefd9754b778c45479c3b72025b1b2 Author: dbarclay Date: 2015-03-24T17:20:27Z sep.: =-=-=-=-=-=-=-= forkCount 3 -> 2. [/pom.xml] =-=-=-=-=-=-=-= >= in Review, < awaiting Merge Conflicts: pom.xml commit 3e155dc1345118a997de66b338adead8328517bd Author: dbarclay Date: 2015-06-20T02:05:39Z DRILL-3151: Fix many ResultSetMetaData method return values. Added ~unit test for ResultSetMetaData implementation. Made getObject return classes available to implementation of getColumnClassName: - Added SqlAccessor.getObjectClass() (to put that metadata right next to code to which it corresponds rather than in far-away parallel code). - Added similar AvaticaDrillSqlAccessor.getObjectClass(). - Changed DrillAccessorList.accessors from Accessor[] to AvaticaDrillSqlAccessor[] for better access to JDBC getObject return class. - Extracted return classes from accessors to pass to updateColumnMetaData. Reworked some data type mapping and utilities: - Added Added Types.getSqlTypeName(...). - Renamed Types.getJdbcType(...) to getJdbcTypeCode(...) - Replaced Types.isUnSigned with isJdbcSignedType. - Fixed various bogus RPC-type XXX -> java.sql.Types.SMALLINT mappings. - Removed DrillColumnMetaDataList.getJdbcTypeName. - Moved getAvaticaType up (for bottom-up order). - Revised DrillColumnMetaDataList.getAvaticaType(...). MAIN: - Updated updateColumnMetaData(...) to change many calculations of metadata input to ColumnMetaData construction. [DrillColumnMetaDataList] Updated other metadata tests per changes. commit ceae4668f250672391e51c84d9b4e295a4c0f4a5 Author: dbarclay Date: 2015-07-02T22:04:31Z DRILL-3483: Clarify CommonConstants' constants. [CommonConstants, DrillConfig, PathScanner] Renamed constants. Documented constants. Removed extraneous "public static final" (redudant and abnormal since in interface). commit 03dafaca207b45c01519e22ceeb0dd2784db18d5 Author: dbarclay Date: 2015-04-17T20:09:59Z DRILL-2696: Test for future DRILL-2696 fix (currently disabled with @Ignore). commit 73183a415ac765863ade2a632799f328fcddad74 Author: dbarclay Date: 2015-04-17T23:27:46Z DRILL-2815: Some PathScanner logging, misc. cleanup. Add some DEBUG-level log calls and augmented, edited log message. Misc. code "hygiene": - Added method doc comment. - Renamed to clarify a number of names. - Added "final". - Fixed indentation; wrapped some long lines. commit c68cc505c03b06b2db3d18eef7bce965408efb15 Author: dbarclay Date: 2015-03-29T21:46:47Z temp: logging adjustments. [exec/java-exec/src/test/resources/logback.xml] commit e48f5267c6c1b27cd2fa500a8062876a61e0f6d2 Author: dbarclay Date: 2015-03-29T21:47:11Z temp: logging adjustments. [exec/jdbc/src/test/resources/logback.xml] commit a0e9cb671e08ded93e571fa096fa5c38f765ed85 Author: dbarclay Date: 2015-04-06T18:11:13Z temp: logging adjustments. [common/src/test/resources/logback.xml] commit 321b9d09da0973e8da48f81a94c64b3503602321 Author: dbarclay Date: 2015-05-10T19:33:21Z temp: logging adjustments. [distribution/src/resources/logback.xml] commit d7da49862c13144c29d78d49c9d4605952f436cf Author: dbarclay Date: 2015-05-14T22:40:06Z temp: logging adjustments (probably TEMP). [exec/jdbc/src/test/resources/logback.xml] commit ae797565156fbaefa795ca22d75631763bd4cfe6 Author: dbarclay Date: 2015-05-15T03:54:51Z temp: logging adjustments. [jdbc/src/test/resources/logback.xml] commit 1cd642ad80311850c4df1f76099a05c78b8a6a31 Author: dbarclay Date: 2015-04-17T20:54:22Z : Logging: Added calls. [DrillConfig] commit 11a72689afc4e3c86cc1dcf6fbd5213040cf444e Author: dbarclay Date: 2015-05-30T18:11:13Z temp: logging adjustments. [jdbc/src/test/resources/logback.xml] commit d7554c584242434d834640dd746c5fc7bfb4c812 Author: dbarclay Date: 2015-05-31T00:45:12Z temp: logging adjustments. [java-exec/src/test/resources/logback.xml] commit 6179813f4c7c149f1d9055842f14d097c3c24bc2
[GitHub] drill pull request: Bugs/various work re master 07 09
Github user dsbos closed the pull request at: https://github.com/apache/drill/pull/87 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3483: Clarify CommonConstants' constants...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/88 DRILL-3483: Clarify CommonConstants' constants. Renamed constants. Documented constants. Removed extraneous "public static final" (redundant and abnormal since in interface). You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-3483-CommonConstants-clarification Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/88.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #88 commit 6e39eb7ca428d0d0baeeb29f50ed25b1a8872685 Author: dbarclay Date: 2015-07-02T22:04:31Z DRILL-3483: Clarify CommonConstants' constants. [CommonConstants, DrillConfig, PathScanner] Renamed constants. Documented constants. Removed extraneous "public static final" (redudant and abnormal since in interface). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3483: Clarify CommonConstants' constants...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/88#issuecomment-121101705 Ping? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: Enhanced logging and associated code hygiene
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/93 Enhanced logging and associated code hygiene You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-2815-etc-Logging Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/93.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #93 commit 24390a34e6886d019382f0e0c00cab570a499045 Author: dbarclay Date: 2015-07-14T04:02:40Z DRILL-2815: Augment PathScanner logging; clean up code. Added some DEBUG- and TRACE-level log calls. Augmented/edited existing log call. Misc. code hygiene: - Added documentation of forResources(...). - Renamed to clarify a number of local names. - Made logger private. - Added "final". - Fixed indentation; wrapped some long lines; etc. Conflicts: common/src/main/java/org/apache/drill/common/util/PathScanner.java commit e740ac4f7acc022922b5e5eebe209de42c504406 Author: dbarclay Date: 2015-07-14T19:11:36Z DRILL-3496: Augment logging in DrillConfig and classpath scanning. Main: Added and augmented logging calls. Code Hygiene: - Added/revised doc. comments, code comments. - Renamed some locals. - Added "final". - Wrapped lines. Also fixed currentTimeMillis -> Stopwatch in Drillbit. Conflicts: common/src/main/java/org/apache/drill/common/config/DrillConfig.java --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: Enhanced logging and associated code hygiene
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/93#discussion_r34642779 --- Diff: common/src/main/java/org/apache/drill/common/config/DrillConfig.java --- @@ -44,26 +46,33 @@ import com.google.common.collect.ImmutableList; import com.typesafe.config.Config; import com.typesafe.config.ConfigFactory; +import com.typesafe.config.ConfigRenderOptions; public final class DrillConfig extends NestedConfig{ -// private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillConfig.class); + private static final Logger logger = getLogger(DrillConfig.class); --- End diff -- To get rid of the unnecessarily verbose qualified names in that boilerplate code (which, when having any unnecessary verbosity, adds unnecessary risk of not noticing mistakes like this: public class TestMultiInputAdd extends PopUnitTestBase { //private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestMathFunctions.class); ) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: Enhanced logging and associated code hygiene
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/93#discussion_r34649690 --- Diff: common/src/main/java/org/apache/drill/common/config/DrillConfig.java --- @@ -138,34 +155,77 @@ public static DrillConfig create(Properties testConfigurations) { return create(null, testConfigurations, true); } - public static DrillConfig create(String overrideFileName, boolean enableServerConfigs) { -return create(overrideFileName, null, enableServerConfigs); + /** + * ... + * @param overrideFileResourcePathname + * see {@link #create(String)}'s {@code overrideFileResourcePathname} + */ + public static DrillConfig create(String overrideFileResourcePathname, boolean enableServerConfigs) { +return create(overrideFileResourcePathname, null, enableServerConfigs); } - private static DrillConfig create(String overrideFileName, Properties overriderProps, boolean enableServerConfigs) { -overrideFileName = overrideFileName == null ? CommonConstants.CONFIG_OVERRIDE : overrideFileName; - -// first we load defaults. + /** + * ... + * @param overrideFileResourcePathname + * see {@link #create(String)}'s {@code overrideFileResourcePathname} + * @param overriderProps + * optional property map for further overriding (after override file + * is assimilated + * @param enableServerConfigs + * whether to enable server-specific configuration options + * @return + */ + private static DrillConfig create(String overrideFileResourcePathname, +final Properties overriderProps, +final boolean enableServerConfigs) { +overrideFileResourcePathname = +overrideFileResourcePathname == null +? CommonConstants.CONFIG_OVERRIDE +: overrideFileResourcePathname; + +// 1. Load defaults configuration file. Config fallback = null; final ClassLoader[] classLoaders = ClasspathHelper.classLoaders(); for (ClassLoader classLoader : classLoaders) { - if (classLoader.getResource(CommonConstants.CONFIG_DEFAULT) != null) { -fallback = ConfigFactory.load(classLoader, CommonConstants.CONFIG_DEFAULT); + final URL url = + classLoader.getResource(CommonConstants.CONFIG_DEFAULT); + if (null != url) { +logger.debug("Loading base config. file at {}.", url); +fallback = +ConfigFactory.load(classLoader, + CommonConstants.CONFIG_DEFAULT); break; } } +// 2. Load per-module configuration files. Collection urls = PathScanner.getConfigURLs(); logger.debug("Loading configs at the following URLs {}", urls); for (URL url : urls) { + logger.debug("Loading module config. file at {}.", url); --- End diff -- First call removed in update. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: Enhanced logging and associated code hygiene
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/93#discussion_r34650500 --- Diff: common/src/main/java/org/apache/drill/common/config/DrillConfig.java --- @@ -138,34 +155,77 @@ public static DrillConfig create(Properties testConfigurations) { return create(null, testConfigurations, true); } - public static DrillConfig create(String overrideFileName, boolean enableServerConfigs) { -return create(overrideFileName, null, enableServerConfigs); + /** + * ... + * @param overrideFileResourcePathname + * see {@link #create(String)}'s {@code overrideFileResourcePathname} + */ + public static DrillConfig create(String overrideFileResourcePathname, boolean enableServerConfigs) { +return create(overrideFileResourcePathname, null, enableServerConfigs); } - private static DrillConfig create(String overrideFileName, Properties overriderProps, boolean enableServerConfigs) { -overrideFileName = overrideFileName == null ? CommonConstants.CONFIG_OVERRIDE : overrideFileName; - -// first we load defaults. + /** + * ... + * @param overrideFileResourcePathname + * see {@link #create(String)}'s {@code overrideFileResourcePathname} + * @param overriderProps + * optional property map for further overriding (after override file + * is assimilated + * @param enableServerConfigs + * whether to enable server-specific configuration options + * @return + */ + private static DrillConfig create(String overrideFileResourcePathname, +final Properties overriderProps, +final boolean enableServerConfigs) { +overrideFileResourcePathname = +overrideFileResourcePathname == null +? CommonConstants.CONFIG_OVERRIDE +: overrideFileResourcePathname; + +// 1. Load defaults configuration file. Config fallback = null; final ClassLoader[] classLoaders = ClasspathHelper.classLoaders(); for (ClassLoader classLoader : classLoaders) { - if (classLoader.getResource(CommonConstants.CONFIG_DEFAULT) != null) { -fallback = ConfigFactory.load(classLoader, CommonConstants.CONFIG_DEFAULT); + final URL url = + classLoader.getResource(CommonConstants.CONFIG_DEFAULT); + if (null != url) { +logger.debug("Loading base config. file at {}.", url); +fallback = +ConfigFactory.load(classLoader, + CommonConstants.CONFIG_DEFAULT); break; } } +// 2. Load per-module configuration files. Collection urls = PathScanner.getConfigURLs(); logger.debug("Loading configs at the following URLs {}", urls); for (URL url : urls) { + logger.debug("Loading module config. file at {}.", url); fallback = ConfigFactory.parseURL(url).withFallback(fallback); } -Config effectiveConfig = ConfigFactory.load(overrideFileName).withFallback(fallback); +// 3. Load any overrides configuration file and overrides from JVM --- End diff -- That "overrides" is part of the noun phrase "overrides configuration file" (parallel to "defaults configuration file" and "per-module configuration file") (not a reference to the overrides from any such file). Yes, it's a bit ambiguous between meaning that and seeming like a word is missing if one doesn't see the other phrases. (Changing the "any" to "the" might reduce that ambiguity, but then it would imply that that code expects there to be an overrides configuration file, when it's actually optional.) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: Enhanced logging and associated code hygiene
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/93#issuecomment-121513460 > ... clutter Lilith ... 3) Every other log statement in your patch should be TRACE level. ... Commit 872309a... reverts to one log call and log entry per list, but retains the improved readability by putting each listed item on a separate line. > ... 1) Overrides in DrillConfig (step 3 and 4 in DrillConfig#create) could be at INFO level. ... Since the set of per-module configuration files is affected by user action (installing plug-ins), step 2's logging should probably be at the same level. And then, since that's reporting every configuration file except the defaults file, maybe the defaults file should also be listed at the same level. Comments? (INFO for all those config files?) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2818, DRILL-3496: Enhanced logging and a...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/93#discussion_r35052392 --- Diff: common/src/main/java/org/apache/drill/common/config/DrillConfig.java --- @@ -90,7 +97,8 @@ public DrillConfig(Config config, boolean enableServer) { } /** - * Create a DrillConfig object using the default config file name + * Creates a DrillConfig object using the default config file name + * and with server-specific configuration options disabled. --- End diff -- Fixed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2818, DRILL-3496: Enhanced logging and a...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/93#discussion_r35052980 --- Diff: common/src/main/java/org/apache/drill/common/config/DrillConfig.java --- @@ -138,34 +155,77 @@ public static DrillConfig create(Properties testConfigurations) { return create(null, testConfigurations, true); } - public static DrillConfig create(String overrideFileName, boolean enableServerConfigs) { -return create(overrideFileName, null, enableServerConfigs); + /** + * ... + * @param overrideFileResourcePathname + * see {@link #create(String)}'s {@code overrideFileResourcePathname} + */ + public static DrillConfig create(String overrideFileResourcePathname, boolean enableServerConfigs) { +return create(overrideFileResourcePathname, null, enableServerConfigs); } - private static DrillConfig create(String overrideFileName, Properties overriderProps, boolean enableServerConfigs) { -overrideFileName = overrideFileName == null ? CommonConstants.CONFIG_OVERRIDE : overrideFileName; - -// first we load defaults. + /** + * ... + * @param overrideFileResourcePathname + * see {@link #create(String)}'s {@code overrideFileResourcePathname} + * @param overriderProps + * optional property map for further overriding (after override file + * is assimilated + * @param enableServerConfigs + * whether to enable server-specific configuration options + * @return + */ + private static DrillConfig create(String overrideFileResourcePathname, +final Properties overriderProps, +final boolean enableServerConfigs) { +overrideFileResourcePathname = +overrideFileResourcePathname == null +? CommonConstants.CONFIG_OVERRIDE +: overrideFileResourcePathname; + +// 1. Load defaults configuration file. Config fallback = null; final ClassLoader[] classLoaders = ClasspathHelper.classLoaders(); for (ClassLoader classLoader : classLoaders) { - if (classLoader.getResource(CommonConstants.CONFIG_DEFAULT) != null) { -fallback = ConfigFactory.load(classLoader, CommonConstants.CONFIG_DEFAULT); + final URL url = + classLoader.getResource(CommonConstants.CONFIG_DEFAULT); + if (null != url) { +logger.debug("Loading base config. file at {}.", url); +fallback = +ConfigFactory.load(classLoader, + CommonConstants.CONFIG_DEFAULT); break; } } +// 2. Load per-module configuration files. Collection urls = PathScanner.getConfigURLs(); logger.debug("Loading configs at the following URLs {}", urls); for (URL url : urls) { + logger.debug("Loading module config. file at {}.", url); fallback = ConfigFactory.parseURL(url).withFallback(fallback); } -Config effectiveConfig = ConfigFactory.load(overrideFileName).withFallback(fallback); +// 3. Load any overrides configuration file and overrides from JVM --- End diff -- Reworded a bit. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2818, DRILL-3496: Enhanced logging and a...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/93#discussion_r35053195 --- Diff: common/src/main/java/org/apache/drill/common/config/DrillConfig.java --- @@ -44,31 +46,37 @@ import com.google.common.collect.ImmutableList; import com.typesafe.config.Config; import com.typesafe.config.ConfigFactory; +import com.typesafe.config.ConfigRenderOptions; public final class DrillConfig extends NestedConfig{ -// private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillConfig.class); + private static final Logger logger = getLogger(DrillConfig.class); + private final ObjectMapper mapper; private final ImmutableList startupArguments; public static final boolean ON_OSX = System.getProperty("os.name").contains("OS X"); - @SuppressWarnings("restriction") private static final long MAX_DIRECT_MEMORY = sun.misc.VM.maxDirectMemory(); + @SuppressWarnings("restriction") + private static final long MAX_DIRECT_MEMORY = sun.misc.VM.maxDirectMemory(); @SuppressWarnings("unchecked") private volatile List> sinkQueues = new CopyOnWriteArrayList>(new Queue[1]); @VisibleForTesting - public DrillConfig(Config config, boolean enableServer) { + public DrillConfig(Config config, boolean enableServerConfigs) { super(config); -logger.debug("Setting up config object."); +logger.debug("Setting up DrillConfig object."); +if (logger.isTraceEnabled()) { --- End diff -- No, I didn't expect it to take any significant time; I just thought someone would object if the calls weren't guarded by that if statement. So ... remove it to reduce the clutter, or keep/leave it? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2818, DRILL-3496: Enhanced logging and a...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/93#discussion_r35053589 --- Diff: common/src/main/java/org/apache/drill/common/config/DrillConfig.java --- @@ -115,19 +124,26 @@ public static DrillConfig forClient() { * * Configuration values are retrieved as follows: * - * Check a single copy of "drill-override.conf". If multiple copies are on the classpath, behavior is - * indeterminate. If a non-null value for overrideFileName is provided, this is utilized instead of drill-override.conf. - * Check all copies of "drill-module.conf". Loading order is indeterminate. - * Check a single copy of "drill-default.conf". If multiple copies are on the classpath, behavior is - * indeterminate. + * Check a single copy of "drill-override.conf". If multiple copies are + * on the classpath, which copy is read is indeterminate. + * If a non-null value for overrideFileResourcePathname is provided, this + * is used instead of "{@code drill-override.conf}". + * Check all copies of "{@code drill-module.conf}". Loading order is + * indeterminate. + * Check a single copy of "{@code drill-default.conf}". If multiple + * copies are on the classpath, which copy is read is indeterminate. * * * - * @param overrideFileName The name of the file to use for override purposes. + * @param overrideFileResourcePathname + * the classpath resource pathname of the file to use for + * configuration override purposes; {@code null} specifies to use the + * default pathname ({@link CommonConstants.CONFIG_OVERRIDE}) + * (not to not look for an override file at all) --- End diff -- Reworded. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2818, DRILL-3496: Enhanced logging and a...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/93#discussion_r35053796 --- Diff: common/src/main/java/org/apache/drill/common/logical/FormatPluginConfigBase.java --- @@ -27,11 +27,20 @@ static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(FormatPluginConfigBase.class); - public synchronized static Class[] getSubTypes(DrillConfig config){ -List packages = config.getStringList(CommonConstants.STORAGE_PLUGIN_CONFIG_SCAN_PACKAGES); -Class[] sec = PathScanner.scanForImplementationsArr(FormatPluginConfig.class, packages); -logger.debug("Adding Format Plugin Configs including {}", (Object) sec ); -return sec; + public synchronized static Class[] getSubTypes(DrillConfig config) { +List packages = + config.getStringList(CommonConstants.STORAGE_PLUGIN_CONFIG_SCAN_PACKAGES); +Class[] pluginClasses = +PathScanner.scanForImplementationsArr(FormatPluginConfig.class, packages); +if (logger.isDebugEnabled()) { + final StringBuilder sb = new StringBuilder(); --- End diff -- Yeah, I thought it should be factored out, but it wasn't clear where. (Also, the functionality is close to that of some utility join(...) methods--but not quite the same.) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2818, DRILL-3496: Enhanced logging and a...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/93#discussion_r35054916 --- Diff: common/src/main/java/org/apache/drill/common/config/DrillConfig.java --- @@ -138,34 +154,83 @@ public static DrillConfig create(Properties testConfigurations) { return create(null, testConfigurations, true); } - public static DrillConfig create(String overrideFileName, boolean enableServerConfigs) { -return create(overrideFileName, null, enableServerConfigs); + /** + * ... + * @param overrideFileResourcePathname + * see {@link #create(String)}'s {@code overrideFileResourcePathname} + */ + public static DrillConfig create(String overrideFileResourcePathname, boolean enableServerConfigs) { +return create(overrideFileResourcePathname, null, enableServerConfigs); } - private static DrillConfig create(String overrideFileName, Properties overriderProps, boolean enableServerConfigs) { -overrideFileName = overrideFileName == null ? CommonConstants.CONFIG_OVERRIDE : overrideFileName; - -// first we load defaults. + /** + * ... --- End diff -- [Reply via e-mail because I don't currently see this comment of yours on the pull-request page.] > > + /** > > + * ...xx > I haven't seen this convention before, but I assume this means that the docs for similar methods suffice and don't need to be repeated here. No, actually it was an attempt to indicate that I hadn't addressed the method description (the part before the parameters), and not intended to mean "otherwise same as above". I was trying to quickly at least document the parameter (add the reference to the other existing text) without taking time to fill in the whole documentation comment. Would simply leaving the method description part blank be better than using "..."? Daniel Jason Altekruse wrote: > > In common/src/main/java/org/apache/drill/common/config/DrillConfig.java <https://github.com/apache/drill/pull/93#discussion_r35042277>: > > >} > > > > - private static DrillConfig create(String overrideFileName, Properties overriderProps, boolean enableServerConfigs) { > > -overrideFileName = overrideFileName == null ? CommonConstants.CONFIG_OVERRIDE : overrideFileName; > > - > > -// first we load defaults. > > + /** > > + * ... > > I haven't seen this convention before, but I assume this means that the docs for similar methods suffice and don't need to be repeated here. I would possibly recommend moving the longer comment above to here, in which case I think it would be possible to just put "..." above all of the alternative implementations of this method. I think it makes the most sense to put the comment describing the general idea of the functions purpose above the version that also describes what all of parameters can do. > > â > Reply to this email directly or view it on GitHub <https://github.com/apache/drill/pull/93/files#r35042277>. > -- Daniel Barclay MapR Technologies --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2818, DRILL-3496: Enhanced logging and a...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/93#discussion_r35136041 --- Diff: common/src/main/java/org/apache/drill/common/config/DrillConfig.java --- @@ -138,34 +154,76 @@ public static DrillConfig create(Properties testConfigurations) { return create(null, testConfigurations, true); } - public static DrillConfig create(String overrideFileName, boolean enableServerConfigs) { -return create(overrideFileName, null, enableServerConfigs); + /** + * ... + * @param overrideFileResourcePathname + * see {@link #create(String)}'s {@code overrideFileResourcePathname} + */ + public static DrillConfig create(String overrideFileResourcePathname, boolean enableServerConfigs) { +return create(overrideFileResourcePathname, null, enableServerConfigs); } - private static DrillConfig create(String overrideFileName, Properties overriderProps, boolean enableServerConfigs) { -overrideFileName = overrideFileName == null ? CommonConstants.CONFIG_OVERRIDE : overrideFileName; - -// first we load defaults. + /** + * ... + * @param overrideFileResourcePathname + * see {@link #create(String)}'s {@code overrideFileResourcePathname} + * @param overriderProps + * optional property map for further overriding (after override file + * is assimilated + * @param enableServerConfigs + * whether to enable server-specific configuration options + * @return + */ + private static DrillConfig create(String overrideFileResourcePathname, +final Properties overriderProps, +final boolean enableServerConfigs) { +overrideFileResourcePathname = +overrideFileResourcePathname == null +? CommonConstants.CONFIG_OVERRIDE +: overrideFileResourcePathname; + +// 1. Load defaults configuration file. Config fallback = null; final ClassLoader[] classLoaders = ClasspathHelper.classLoaders(); for (ClassLoader classLoader : classLoaders) { - if (classLoader.getResource(CommonConstants.CONFIG_DEFAULT) != null) { -fallback = ConfigFactory.load(classLoader, CommonConstants.CONFIG_DEFAULT); + final URL url = + classLoader.getResource(CommonConstants.CONFIG_DEFAULT); + if (null != url) { +logger.debug("Loading base configuration file at {}.", url); +fallback = +ConfigFactory.load(classLoader, + CommonConstants.CONFIG_DEFAULT); break; } } +// 2. Load per-module configuration files. Collection urls = PathScanner.getConfigURLs(); -logger.debug("Loading configs at the following URLs {}", urls); for (URL url : urls) { + logger.debug("Loading module configuration file at {}.", url); --- End diff -- Done. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2818, DRILL-3496: Enhanced logging and a...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/93#discussion_r35136191 --- Diff: common/src/main/java/org/apache/drill/common/config/DrillConfig.java --- @@ -44,31 +46,37 @@ import com.google.common.collect.ImmutableList; import com.typesafe.config.Config; import com.typesafe.config.ConfigFactory; +import com.typesafe.config.ConfigRenderOptions; public final class DrillConfig extends NestedConfig{ -// private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillConfig.class); + private static final Logger logger = getLogger(DrillConfig.class); + private final ObjectMapper mapper; private final ImmutableList startupArguments; public static final boolean ON_OSX = System.getProperty("os.name").contains("OS X"); - @SuppressWarnings("restriction") private static final long MAX_DIRECT_MEMORY = sun.misc.VM.maxDirectMemory(); + @SuppressWarnings("restriction") + private static final long MAX_DIRECT_MEMORY = sun.misc.VM.maxDirectMemory(); @SuppressWarnings("unchecked") private volatile List> sinkQueues = new CopyOnWriteArrayList>(new Queue[1]); @VisibleForTesting - public DrillConfig(Config config, boolean enableServer) { + public DrillConfig(Config config, boolean enableServerConfigs) { super(config); -logger.debug("Setting up config object."); +logger.debug("Setting up DrillConfig object."); +if (logger.isTraceEnabled()) { --- End diff -- Removed isTraceEnabled guard. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2818, DRILL-3496: Enhanced logging and a...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/93#discussion_r35279254 --- Diff: common/src/main/java/org/apache/drill/common/config/DrillConfig.java --- @@ -138,34 +154,83 @@ public static DrillConfig create(Properties testConfigurations) { return create(null, testConfigurations, true); } - public static DrillConfig create(String overrideFileName, boolean enableServerConfigs) { -return create(overrideFileName, null, enableServerConfigs); + /** + * ... + * @param overrideFileResourcePathname + * see {@link #create(String)}'s {@code overrideFileResourcePathname} + */ + public static DrillConfig create(String overrideFileResourcePathname, boolean enableServerConfigs) { +return create(overrideFileResourcePathname, null, enableServerConfigs); } - private static DrillConfig create(String overrideFileName, Properties overriderProps, boolean enableServerConfigs) { -overrideFileName = overrideFileName == null ? CommonConstants.CONFIG_OVERRIDE : overrideFileName; - -// first we load defaults. + /** + * ... --- End diff -- > I'm not even sure that it makes sense to document these methods individually. If a method is not documented individually, then viewing that method's documentation isn't going to yield much information--remember that people don't see comments just in the context of the surrounding source code, but also in IDEs (e.g., showing one method's documentation at a time) and in generated Javadoc pages. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3153: Fix JDBC's getIdentifierQuoteStrin...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/99 DRILL-3153: Fix JDBC's getIdentifierQuoteString() to report Drill's backquote. Added override of Avatica's default implementation returning SQL standard value. Added Javadoc. Added unit test. Also moved unit test for nullsAreSortedXxx methods. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-3153-getIdentifierQuoteString Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/99.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #99 commit 299fc974aca30e20cda65102b83fd7f071105e27 Author: dbarclay Date: 2015-07-24T00:46:05Z DRILL-3153: Fix JDBC's getIdentifierQuoteString() to report Drill's backquote. Added override of Avatica's default implementation returning SQL standard value. Added Javadoc. Added unit test. Also moved unit test for nullsAreSortedXxx methods. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2818, DRILL-3496: Enhanced logging and a...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/93#issuecomment-125272213 (More "final" added in already touched methods.) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2818, DRILL-3496: Enhanced logging and a...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/93#discussion_r35559349 --- Diff: common/src/main/java/org/apache/drill/common/logical/FormatPluginConfigBase.java --- @@ -27,11 +27,20 @@ static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(FormatPluginConfigBase.class); - public synchronized static Class[] getSubTypes(DrillConfig config){ -List packages = config.getStringList(CommonConstants.STORAGE_PLUGIN_CONFIG_SCAN_PACKAGES); -Class[] sec = PathScanner.scanForImplementationsArr(FormatPluginConfig.class, packages); -logger.debug("Adding Format Plugin Configs including {}", (Object) sec ); -return sec; + public synchronized static Class[] getSubTypes(DrillConfig config) { +List packages = + config.getStringList(CommonConstants.STORAGE_PLUGIN_CONFIG_SCAN_PACKAGES); +Class[] pluginClasses = +PathScanner.scanForImplementationsArr(FormatPluginConfig.class, packages); +if (logger.isDebugEnabled()) { + final StringBuilder sb = new StringBuilder(); --- End diff -- Reduce line count by factoring out part that Joiner could be used for. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3566: Fix: PreparedStatement.executeQuer...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/101 DRILL-3566: Fix: PreparedStatement.executeQuery() got ClassCastException Main: Restored DrillResultSetImpl(...)'s statement parameter from overly restrictive DrillStatementImpl to AvaticaStatement and removed caller cast that was throwing. (Relatedly, adjusted getStatement() and moved internal casting from statement to connection.) Added basic test of querying via Prepared Statement. [PreparedStatementTest] Added some case test of statement-creation methods. [Connection Test] You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-3566 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/101.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #101 commit a3b979734b0ead26e3fb903104d9b65f8464b663 Author: dbarclay Date: 2015-07-28T02:27:50Z DRILL-3566: Fix: PreparedStatement.executeQuery() got ClassCastException. Main: Restored DrillResultSetImpl(...)'s statement parameter from overly restrictive DrillStatementImpl to AvaticaStatement and removed caller cast that was throwing. (Relatedly, adjusted getStatement() and moved internal casting from statement to connection.) Added basic test of querying via PreparedStatement. [PreparedStatementTest] Added some case test of statement-creation methods. [ConnectionTest] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2818, DRILL-3496: Enhanced logging and a...
Github user dsbos closed the pull request at: https://github.com/apache/drill/pull/93 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3566: PreparedStatement fix and DRILL-33...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/111 DRILL-3566: PreparedStatement fix and DRILL-3347: ...hadoop.io.Text fix You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-3566_etc._jdbc Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/111.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #111 commit aad3db5dd3787e0c0e26b42e02a2351d1d5df788 Author: dbarclay Date: 2015-07-28T02:27:50Z DRILL-3566: Fix: PreparedStatement.executeQuery() got ClassCastException. Main: Restored DrillResultSetImpl(...)'s statement parameter from overly restrictive DrillStatementImpl to AvaticaStatement and removed caller cast that was throwing. (Relatedly, adjusted getStatement() and moved internal casting from statement to connection.) Added basic test of querying via PreparedStatement. [PreparedStatementTest] Added some case test of statement-creation methods. [ConnectionTest] commit 537f7db912f8a80088b331bde67cefbc132fae5a Author: dbarclay Date: 2015-08-04T23:51:07Z DRILL-3347: VARCHAR ResultSet.getObject returned ...hadoop.io.Text, not String. Core fix: - Fixed {,Nullable}VarCharAccessor's getObject() to return String instead of value vector's internal org.apache.hadoop.io.Text. - Updated unit tests (to expect only String now). [DatabaseMetaDataGetColumnsTest, ResultSetMetaDataTest] Also Added getObject check in tracing proxy test. [TracingProxyDriverTest] Changed hard references to Hadoop's Text and JodaTime's Period to strings in warning check in tracing proxy. [InvocationReporterImpl] Cleanup: - Added @Override annotations. [SqlAccessors] - (Unintentionally) fixed (undetected) missing comma. [ValueVectorTypes.tdd] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3566: Fix: PreparedStatement.executeQuer...
Github user dsbos closed the pull request at: https://github.com/apache/drill/pull/101 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3641: Doc. RecordBatch.IterOutcome (enum...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/113 DRILL-3641: Doc. RecordBatch.IterOutcome (enumerators and possible sequences). Documented RecordBatch.IterOutcome (RecordBatch.next() protocol) much more. Also moved AbstractRecordBatch.BatchState's documentation test from non-documentation comments to documentation comments. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-3641 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/113.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #113 commit 5ca94981442ea0a899c690ff1471369618a5471d Author: dbarclay Date: 2015-07-31T01:54:59Z DRILL-3641: Doc. RecordBatch.IterOutcome (enumerators and possible sequences). Documented RecordBatch.IterOutcome (RecordBatch.next() protocol) much more. Also moved AbstractRecordBatch.BatchState's documentation test from non-documentation comments to documentation comments. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3641: Doc. RecordBatch.IterOutcome (enum...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/113#issuecomment-131672321 > Would be good to note here the purpose. ... Done. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3641: Doc. RecordBatch.IterOutcome (enum...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/113#issuecomment-131674681 Hey, what happens (what is a caller supposed to do) if the caller receives NOT_YET but the caller has no local work to do? (Re-calling next() immediately would yield a spin wait, so presumably we avoid that if possible. A timed wait would involve a (probably) arbitrary time, so presumably we avoid that if possible. (Delegating to the next-level caller by returning NOT_YET just devolves back to this same question.) Is there another general option (general in the sense of callers and callees that don't know anything about each other)? Or is it that NOT_YET is used only between pairs of callers and callee that know enough about each other to coordinate somehow so they can avoid spin waits and aribitrary delays?) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3589: Update JDBC driver to shade and mi...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/116#issuecomment-131970908 It seems that you have shaded references to every Java package other than org.apache.drill.** (not just for every package from every depended-on artifact, but for _every_ package--including JDK packages). Note how the following command says it can't find "oadd/java/sql/Driver": $ java -cp ./exec/jdbc-all/target/drill-jdbc-all-1.2.0-SNAPSHOT.jar org.apache.drill.jdbc.Driver Exception in thread "main" java.lang.NoClassDefFoundError: oadd/java/sql/Driver ... It seems that in trying to load org.apache.drill.jdbc.Driver it tried to load its base class, normally java.sql.Driver from the JDK, but the reference to that base class was shaded to oadd.org.java.sql.Driver, so of course it couldn't be found. (The expected behavior of that command is an error that org.apache.drill.jdbc.Driver doesn't have a main method.) What setup did you use to test the shading? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3548: Add/edit various Javadoc(/etc.) (a...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/118 DRILL-3548: Add/edit various Javadoc(/etc.) (assorted from plugin exploration). Added/edited various Javadoc and code comments. Also: Fixed Javadoc references ("#", imports). Edited a few messages. Wrapped a few lines. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-3548_various_doc Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/118.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #118 commit 90fa3476ab4b988a93b8720219ea3856c7a250dd Author: dbarclay Date: 2015-07-20T20:50:30Z DRILL-3548: Add/edit various Javadoc(/etc.) (assorted from plugin exploration). Added/edited various Javadoc and code comments. Also: Fixed Javadoc references ("#", imports). Edited a few messages. Wrapped a few lines. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3661: Edit JDBC doc. for clarity, more c...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/119 DRILL-3661: Edit JDBC doc. for clarity, more consistency, bug fixes. Also fixed two serialVersionUID literals. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-3661_JDBC_doc Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/119.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #119 commit b0e76a58a4573f6924ecadaa9a649af5f878d5a2 Author: dbarclay Date: 2015-08-18T00:26:45Z DRILL-3661: Edit JDBC doc. for clarity, more consistency, bug fixes. Also fixed two serialVersionUID literals. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3661: Edit JDBC doc. for clarity, more c...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/119#discussion_r38443172 --- Diff: exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionConfig.java --- @@ -22,6 +22,24 @@ import net.hydromatic.avatica.ConnectionConfigImpl; + +// TODO(DRILL-): Change public DrillConnectionConfig from class to interface. --- End diff -- Fixed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3589: Update JDBC driver to shade and mi...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/116#issuecomment-136889943 Something seems to be broken. After rebasing your branch on my branch with my DRILL-3347 (Hadoop Test) and DRILL-3566 (Prep.Stmt.) fixes, I tried installing the resulting JDBC-all Jar file on Spotfire, but Spotfire's getting IndexOutOfBoundsExceptions somewhere within ResultSet.next(). I'll see if I can identify which shading might be causing that or what's going on in next(). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3589: Update JDBC driver to shade and mi...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/116#issuecomment-136890582 Another problem is that Logback logging might be broken (although maybe I just configured it wrong): When I added (on Spotfire's class path) a Jar file containing a logback.xml file in order to turn on logging, instead of getting log messages, I got these messages: SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder". SLF4J: Defaulting to no-operation (NOP) logger implementation SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details. My guess is that whatever module we intended to provide org.slf4j.impl.StaticLoggerBinder isn't included in the JDBC-all Jar file now. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3589: Update JDBC driver to shade and mi...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/116#issuecomment-136902647 Regarding SLF4J and what should or shouldn't be included in the JDBC-all Jar file: Do we intend that someone using the JDBC-all Jar file can turn on Drill JDBC-layer logging by adding to the class path only a logback.xml file (that is, without adding additional Jar files/classes)? If so, then it seems like the JDBC-all Jar file is probably missing the logback-classic Jar file (which contains a org.slf4j.impl.StaticLoggerBinder). Or is it the case that we're leaving out back-end SLF4J libraries/classes on purpose (perhaps to avoid interfering with the user's choice of which back end to use)? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3661: Edit JDBC doc. for clarity, more c...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/119#discussion_r38558416 --- Diff: exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillCursor.java --- @@ -66,17 +69,18 @@ private boolean afterFirstBatch = false; /** - * Whether the next call to this.next() should just return {@code true} rather - * than calling nextRowInternally() to try to advance to the next - * record. + * Whether the next call to {@code this.}{@link #next()} should just return + * {@code true} rather than calling {@link #nextRowInternally()} to try to + * advance to the next record. * - * Currently, can be true only for first call to next(). + * Currently, can be true only for first call to {@link #next()}. * * - * (Relates to loadInitialSchema()'s calling nextRowInternally() + * (Relates to {@link #loadInitialSchema()}'s calling nextRowInternally() --- End diff -- Frequently, for second and later references in the same paragraph or adjacent paragraphs, I avoid making those later references links, since having more links can make the text less readable. However, since here I already have multiple links to next(), I did make that nextRowInternally() a link (and added a missing {@code...} for "Statement.execute...(...)"). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3661: Edit JDBC doc. for clarity, more c...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/119#issuecomment-137176204 Rebased. Edited doc. comment formatting. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3566: PreparedStatement fix and DRILL-33...
Github user dsbos closed the pull request at: https://github.com/apache/drill/pull/111 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3566: Fix: PreparedStatement.executeQuer...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/143 DRILL-3566: Fix: PreparedStatement.executeQuery() got ClassCastException. Main: Restored DrillResultSetImpl(...)'s statement parameter from overly restrictive DrillStatementImpl to AvaticaStatement and removed caller cast that was throwing. (Relatedly, adjusted getStatement() and moved internal casting from statement to connection.) Added basic test of querying via PreparedStatement. [PreparedStatementTest] Added some case test of statement-creation methods. [ConnectionTest] You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-3566 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/143.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #143 commit 1a870538c66fa59070facce7e2a4342d9869b51e Author: dbarclay Date: 2015-07-28T02:27:50Z DRILL-3566: Fix: PreparedStatement.executeQuery() got ClassCastException. Main: Restored DrillResultSetImpl(...)'s statement parameter from overly restrictive DrillStatementImpl to AvaticaStatement and removed caller cast that was throwing. (Relatedly, adjusted getStatement() and moved internal casting from statement to connection.) Added basic test of querying via PreparedStatement. [PreparedStatementTest] Added some case test of statement-creation methods. [ConnectionTest] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3347: VARCHAR ResultSet.getObject return...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/144 DRILL-3347: VARCHAR ResultSet.getObject returned ...hadoop.io.Text, not String. Core fix: - Fixed {,Nullable}VarCharAccessor's getObject() to return String instead of value vector's internal org.apache.hadoop.io.Text. - Updated unit tests (to expect only String now). [DatabaseMetaDataGetColumnsTest, ResultSetMetaDataTest] Also Added getObject check in tracing proxy test. [TracingProxyDriverTest] Changed hard references to Hadoop's Text and JodaTime's Period to strings in warning check in tracing proxy. [InvocationReporterImpl] Cleanup: - Added @Override annotations. [SqlAccessors] - (Unintentionally) fixed (undetected) missing comma. [ValueVectorTypes.tdd] You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-3347 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/144.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #144 commit ccd6ded4387b2b27849762f607a13fa4236351b6 Author: dbarclay Date: 2015-08-04T23:51:07Z DRILL-3347: VARCHAR ResultSet.getObject returned ...hadoop.io.Text, not String. Core fix: - Fixed {,Nullable}VarCharAccessor's getObject() to return String instead of value vector's internal org.apache.hadoop.io.Text. - Updated unit tests (to expect only String now). [DatabaseMetaDataGetColumnsTest, ResultSetMetaDataTest] Also Added getObject check in tracing proxy test. [TracingProxyDriverTest] Changed hard references to Hadoop's Text and JodaTime's Period to strings in warning check in tracing proxy. [InvocationReporterImpl] Cleanup: - Added @Override annotations. [SqlAccessors] - (Unintentionally) fixed (undetected) missing comma. [ValueVectorTypes.tdd] --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3566: Fix: PreparedStatement.executeQuer...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/143#discussion_r38708736 --- Diff: exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillJdbc41Factory.java --- @@ -103,9 +103,9 @@ public DrillResultSetImpl newResultSet(AvaticaStatement statement, TimeZone timeZone) { final ResultSetMetaData metaData = newResultSetMetaData(statement, prepareResult.getColumnList()); -return new DrillResultSetImpl( (DrillStatementImpl) statement, - (DrillPrepareResult) prepareResult, - metaData, timeZone); +return new DrillResultSetImpl(statement, + (DrillPrepareResult) prepareResult, --- End diff -- Simplified. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3566: Fix: PreparedStatement.executeQuer...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/143#discussion_r38709955 --- Diff: exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java --- @@ -850,9 +851,9 @@ public void moveToCurrentRow() throws SQLException { } @Override - public DrillStatementImpl getStatement() { + public AvaticaStatement getStatement() { --- End diff -- One reason for leaving it in (after changing the return type back) is to hold that comment pointing out that this method doesn't call checkNotClosed() as most other methods do. Fixed copy/paste/forgot-to-edit error "close()" to "getStatement()" in comment. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3566: Fix: PreparedStatement.executeQuer...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/143#discussion_r38710714 --- Diff: exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java --- @@ -87,12 +88,12 @@ boolean hasPendingCancelationNotification; - DrillResultSetImpl(DrillStatementImpl statement, AvaticaPrepareResult prepareResult, + DrillResultSetImpl(AvaticaStatement statement, AvaticaPrepareResult prepareResult, ResultSetMetaData resultSetMetaData, TimeZone timeZone) { super(statement, prepareResult, resultSetMetaData, timeZone); -this.statement = statement; +connection = (DrillConnectionImpl) statement.getConnection(); final int batchQueueThrottlingThreshold = -this.getStatement().getConnection().getClient().getConfig().getInt( +connection.getClient().getConfig().getInt( --- End diff -- Yeah, I don't know why the code was like that. Reworked a bit to eliminate redundant calls and code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3566: Fix: PreparedStatement.executeQuer...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/143#discussion_r38710996 --- Diff: exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java --- @@ -1334,8 +1335,6 @@ public String getQueryId() throws SQLException { @Override protected DrillResultSetImpl execute() throws SQLException{ -DrillConnectionImpl connection = (DrillConnectionImpl) statement.getConnection(); - connection.getClient().runQuery(QueryType.SQL, this.prepareResult.getSql(), --- End diff -- I don't know why the code was like that. (Maybe to be symmetric with connection.getDriver(), or maybe from before client was kept?) Eliminated redundant call to getClient(). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3566: Fix: PreparedStatement.executeQuer...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/143#issuecomment-137607549 > can you confirm that this patch passes all our "jdbc" tests ? It passed before the post-review update commit. I'm re-running regular tests now. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3589: Update JDBC driver to shade and mi...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/116#issuecomment-137653774 > Can you start by reviewing my branch? Let's get it right before we try rebasing. I'm confused by that part of your comment. I have been reviewing your branch, and I think I've reviewed the latest (with-test/without-ProGuard) version of it (hence my earlier review comments, and your replies to them). Have I missed a message or am I looking at the wrong thing? (I have been and am looking at this [Pull Request #116](https://github.com/apache/drill/pull/116).) Also, your mention of rebasing sounds like a reference to a preceding request to rebase, but I don't see or recall any mention of rebasing your patch. Looking again, I see that I just skimmed over the class loader that's just for the test. Is it that you wanted more review of that class? Oh--it is that I hadn't indicated that I was done reviewing it because I didn't give a +1 yet? I hadn't done that before because I was waiting for confirmation on my SLF4J question. You have answered that now (below), so the patch seems "plus-oneable" unless the resolution of whatever the confusion is here reveals a problem with the patch. > With regards to slf4j and logging: ... We support any slf4j logging tool but we don't package any. ... > If we include a logging framework, then a user can't use their own for centralized logging... Roger. Yeah, I thought it might be that. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3566: Fix: PreparedStatement.executeQuer...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/143#issuecomment-137794418 > can you confirm that this patch passes all our "jdbc" tests ? Yes, it passes our regular tests. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3589: Update JDBC driver to shade and mi...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/116#issuecomment-137805930 Okay, now I think I see what happened. First of all, you can now ignore that "Something ... broken ..." comment; it's obsolete. (Your patch no longer seems broken.) (At first I thought things didn't work, and added that comment. Then I noticed that I had a local version/build mismatch, and amended the comment to say "hold on; I'm checking again," and tested again. Then everything worked, so I just deleted the comment from the GitHub review. Next time I'll amend rather than delete.) And now I recognize the "rebasing" reference: I didn't mean rebasing like rebasing on the latest version of master. I was just mentioning that rebasing was what I happened to use (as opposed to cherry-picking, merging, or other patching), in case my choice there caused the apparent breakage, to apply patches for DRILL-3347 and DRILL-3566 so I could try you patch with Spotfire (with would have hit the DRILL-3347 and DRILL-3566 bugs). So ... Your patch seems good; Spotfire ran fine with it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3589: Update JDBC driver to shade and mi...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/116#issuecomment-137811250 Would it be better if the JDBC-all Jar file didn't contain a logback.xml file? If the JDBC-all Jar doesn't already have that file, then to configure logback differently with their own logback.xml file, users only have to get their own file on the classpath _somewhere_. However, if the JDBC-all Jar does already have that file, it seems that users will have to get their logback.xml file on the class path _before_ the JDBC-all Jar file (right?). Although sometimes that's trivial, when the class path is set by, say, listing all .jar files in a lib/ directory (e.g., as in Tomcat for Spotfire), controlling the order might not be possible or easy. (A solution to keep the contents of the logback.xml file (as a convenient starting point for the user to copy out and modify) in the JDBC-all JAR file but not really have an active/interfering actual logback.xml file would be to use a modified name (e.g, logback-example.xml).) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3589: Update JDBC driver to shade and mi...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/116#issuecomment-138646368 +1 (non-binding) once logback.xml is removed --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3778: Add missed part of DRILL-3160 (mak...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/158 DRILL-3778: Add missed part of DRILL-3160 (making JDBC Javadoc available). Main: Configured Javadoc generation (title, package groups, version in headers). Added link to JDBC page in Drill documentation site. Edited/fixed some JDBC Javadoc comments. Added explicit SQLConversionOverflowException to throws clauses for Javadoc effect. Added some imports for Javadoc references. Misc.: Fixed a couple Javadoc syntax errors. Fixed POM indentation. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-3778_DRILL-3160_missed Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/158.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #158 commit 24226aebf6274806e8d1a355f4c6b83dedee2e83 Author: dbarclay Date: 2015-09-14T17:54:34Z DRILL-3778: Add missed part of DRILL-3160 (making JDBC Javadoc available). Main: Configured Javadoc generation (title, package groups, version in headers). Added link to JDBC page in Drill documentation site. Edited/fixed some JDBC Javadoc comments. Added explicit SQLConversionOverflowException to throws clauses for Javadoc effect. Added some imports for Javadoc references. Misc.: Fixed a couple Javadoc syntax errors. Fixed POM indentation. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3822: Have PathScanner use own, not thre...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/165 DRILL-3822: Have PathScanner use own, not thread-context, class loader. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-3822 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/165.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #165 commit 605a65780a38da2e1e8e35c521876e68e5901cab Author: dbarclay Date: 2015-09-23T01:04:45Z DRILL-3822: Have PathScanner use own, not thread-context, class loader. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3822: Have PathScanner use own, not thre...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/165#issuecomment-142719537 @jacques-n, can you review and merge this? We (Krystal and I) have verified that this patch fixes the problem seen in SQuirrel. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3822: Have PathScanner use own, not thre...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/166 DRILL-3822: Have PathScanner use own, not thread-context, class loader. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-3822 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/166.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #166 commit 605a65780a38da2e1e8e35c521876e68e5901cab Author: dbarclay Date: 2015-09-23T01:04:45Z DRILL-3822: Have PathScanner use own, not thread-context, class loader. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3822: Have PathScanner use own, not thre...
Github user dsbos closed the pull request at: https://github.com/apache/drill/pull/165 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3822: Have PathScanner use own, not thre...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/166#issuecomment-142724811 @jacques-n, can you review and merge this? We (Krystal and I) have verified that this patch fixes the problem seen in SQuirrel. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3778: Add missed part of DRILL-3160 (mak...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/158#issuecomment-142729309 @jaltekruse, can you review and merge this commit? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3822: Have PathScanner use own, not thre...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/166#issuecomment-142730429 @hakim, can you review and merge this (probably with your fix for DRILL-3874)? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3778: Add missed part of DRILL-3160 (mak...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/158#discussion_r40260511 --- Diff: exec/jdbc/src/main/java/org/apache/drill/jdbc/package-info.java --- @@ -19,7 +19,13 @@ /** * JDBC driver for Drill. * - * Drill's JDBC driver class is {@link org.apache.drill.jdbc.Driver}. + * Drill's JDBC driver class is + * {@link org.apache.drill.jdbc.Driver org.apache.drill.jdbc.Driver}. --- End diff -- Yes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2489: Throw exception from remaining met...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/170 DRILL-2489: Throw exception from remaining methods for closed JDBC objects. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-2769_2489_JDBC_exceptions Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/170.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #170 commit 90bf53dee9407d40065a820a6f0b7a35cf54fbea Author: dbarclay Date: 2015-08-19T23:55:40Z DRILL-2489: Throw exception from remaining methods for closed JDBC objects. Refactored unit test to check all methods per interface. (Replaced individual, static test methods with bulk reflection-based checking.) [Drill2489CallsAfterCloseThrowExceptionsTest] Added DrillResultSetMetaDataImpl. Added method overrides to check state for remaining methods from Connection, Statement, PreparedStatement, ResultSet, ResultSetMetaData and DatabaseMetaData. Also: - renamed checkNotClosed to throwIfClosed. commit fb960d8be27c3a810474caedfe4a0890fd0222ac Author: dbarclay Date: 2015-08-27T21:05:26Z DRILL-2769: Fix most non-SQLException not-supported-yet exceptions. Core: Added (auto-scanning) unit test. [Drill2769UnsupportedReportsUseSqlExceptionTest] Added translation of lots of UnsupportedOperationExceptions (and some RuntimeExceptions) from Avatica code to SQLFeatureNotSupportedExceptions (tons of method overrides). Also: Added explicit bounds checks in ResultSetMetaData methods and checking of last-accessed column in DrillAccessorList.wasNull() (to fix other RuntimeExceptions to SQLExceptions). Added resetting of last-accessed column to fix latent bug in DrillAccessorList. Hygiene: - Renamed some zero-based index/ordinal-position parameters to "...Offset". - Renamed some one-based index/ordinal-position parameters to "...Number". - Renamed DrillAccessorList lastColumn to rowLastColumnOffset; declared explicit logical null value for rowLastColumnOffset. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2489: Throw exception from remaining met...
Github user dsbos closed the pull request at: https://github.com/apache/drill/pull/170 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: Bugs/drill 2769 2489 jdbc exceptions
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/171 Bugs/drill 2769 2489 jdbc exceptions DRILL-2489: Throw exception from remaining methods for closed JDBC objects. DRILL-2769: Fix most non-SQLException not-supported-yet exceptions. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-2769_2489_JDBC_exceptions Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/171.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #171 commit 90bf53dee9407d40065a820a6f0b7a35cf54fbea Author: dbarclay Date: 2015-08-19T23:55:40Z DRILL-2489: Throw exception from remaining methods for closed JDBC objects. Refactored unit test to check all methods per interface. (Replaced individual, static test methods with bulk reflection-based checking.) [Drill2489CallsAfterCloseThrowExceptionsTest] Added DrillResultSetMetaDataImpl. Added method overrides to check state for remaining methods from Connection, Statement, PreparedStatement, ResultSet, ResultSetMetaData and DatabaseMetaData. Also: - renamed checkNotClosed to throwIfClosed. commit fb960d8be27c3a810474caedfe4a0890fd0222ac Author: dbarclay Date: 2015-08-27T21:05:26Z DRILL-2769: Fix most non-SQLException not-supported-yet exceptions. Core: Added (auto-scanning) unit test. [Drill2769UnsupportedReportsUseSqlExceptionTest] Added translation of lots of UnsupportedOperationExceptions (and some RuntimeExceptions) from Avatica code to SQLFeatureNotSupportedExceptions (tons of method overrides). Also: Added explicit bounds checks in ResultSetMetaData methods and checking of last-accessed column in DrillAccessorList.wasNull() (to fix other RuntimeExceptions to SQLExceptions). Added resetting of last-accessed column to fix latent bug in DrillAccessorList. Hygiene: - Renamed some zero-based index/ordinal-position parameters to "...Offset". - Renamed some one-based index/ordinal-position parameters to "...Number". - Renamed DrillAccessorList lastColumn to rowLastColumnOffset; declared explicit logical null value for rowLastColumnOffset. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3848: Increase timeout time on several t...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/174 DRILL-3848: Increase timeout time on several tests that time out frequently. Tests: - TestTpchDistributedConcurrent - TestExampleQueries - TestFunctionsQuery You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-3848_extend_test_timeouts Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/174.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #174 commit 1ac417dca34f42f0f33c07795665c0579810417d Author: dbarclay Date: 2015-09-27T20:43:57Z DRILL-3848: Increase timeout time on several tests that time out frequently. Tests: - TestTpchDistributedConcurrent - TestExampleQueries - TestFunctionsQuery --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2961: Throw SQLException when attempting...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/71#issuecomment-143905747 This has been merged (commits 1dcf6cfb0d52435bcc7d1f4eada48fc0ce366dc4 and a3ec52a721860a966dfa351f719458a200b27cbf). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2961: Throw SQLException when attempting...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/71#issuecomment-143908786 @kkhatua, Can you close this pull request? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix: No OK_NEW_SCHEMA for 0-row so...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/175 DRILL-2288: Fix: No OK_NEW_SCHEMA for 0-row source; missing downstream schema. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-2288-3641-3659_no-rows_schema Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/175.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #175 commit eee7c29785b1ae6545c2e02aa7676aa4509e1963 Author: dbarclay Date: 2015-09-27T20:55:21Z DRILL-3641: Doc. RecordBatch.IterOutcome (enumerators and possible sequences). [RecordBatch, AbstractRecordBatch] Documented RecordBatch.IterOutcome (RecordBatch.next() protocol) much more. Also moved AbstractRecordBatch.BatchState's documentation text from non-documentation comments to documentation comments. commit ec5a9d0af8647af1472591549be1a9964f6a9151 Author: dbarclay Date: 2015-09-27T20:49:45Z DRILL-2288: Fix: No OK_NEW_SCHEMA for 0-row source; missing downstream schema. Core fix: - Fixed ScanBatch.Mutator's isNewSchema() to stop reporting "new schema" spuriously at end of JSON file with complex types. - Fixed ScanBatch.next() to return OK_NEW_SCHEMA instead of NONE for certain case of zero-row source/reader with new schema. - Fixed UnionAllRecordBatch to handle correct IterOutcome sequence from now-corrected ScanBatch.next(). (Was DRILL-3659.) Also: - Added unit test (checking involved metadata at JDBC level). [Drill2288GetColumnsMetadataWhenNoRowsTest, empty.json] - Enhanced IteratorValidatorBatchIterator, mainly to validate IterOutcome value sequence from next(). - Documented SchemaChangeCallBack and renamed method for clarity. [SchemaChangeCallBack, ScanBatch, AbstractSingleRecordBatch] - Other [ScanBatch]: - Added/edited various code and doc. comments. - Renamed boolean schemaChange -> schemaChanged. - Various code cleanup (e.g., "final", SuppressWarnings, whitespace). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3589: Update JDBC driver to shade and mi...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/116#issuecomment-143913643 @jacques-n, can you close this pull request (given that that 4e3b7dc0333a01e72d0ea9256331ea1e1dd51181 is in)? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix: No OK_NEW_SCHEMA for 0-row so...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/175#issuecomment-152326981 Closing in part to move to new branch. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix: No OK_NEW_SCHEMA for 0-row so...
Github user dsbos closed the pull request at: https://github.com/apache/drill/pull/175 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/228 DRILL-2288: Fix ScanBatch violation of IterOutcome protocol and downstream chain of bugs You can merge this pull request into a Git repository by running: $ git pull https://github.com/dsbos/incubator-drill bugs/drill-2288_etc Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/228.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #228 commit faefc960f1a36cf25fa0b553bab75e1c3fc71222 Author: dbarclay Date: 2015-10-28T02:25:25Z 2288: Pt. 1 Core: Added unit test. [Drill2288GetColumnsMetadataWhenNoRowsTest, empty.json] commit 396a41b155d1ad9413897ae4d87db2337642a1ea Author: dbarclay Date: 2015-11-01T03:36:12Z 2288: Pt. 1 Core: Changed HBase test table #1's # of regions from 1 to 2. [HBaseTestsSuite] Also added TODO(DRILL-3954) comment about # of regions. commit a0fe6b0787c284cda2006355e507c7caa6cae2a7 Author: dbarclay Date: 2015-10-28T02:35:11Z 2288: Pt. 2 Core: Documented IterOutcome much more clearly. [RecordBatch] Also edited some related Javadoc. commit 24b3f4df90711b630b30fe4e2ad68ff798e5731a Author: dbarclay Date: 2015-10-28T02:41:04Z 2288: Pt. 2 Hyg.: Edited doc., added @Override, etc. [AbstractRecordBatch, RecordBatch] Purged unused SetupOutcome. Added @Override. Edited comments. Fix some comments to doc. comments. commit a3108ab18e5f99e9d18f57786b2efa717a61c432 Author: dbarclay Date: 2015-10-28T03:00:26Z 2288: Pt. 3 Core&Hyg.: Added validation of IterOutcome sequence. [IteratorValidatorBatchIterator] Also: Renamed internal members for clarity. Added comments. commit c8fc4b3f5c3df5871a9b07b8f4ae800ddbe0ce64 Author: dbarclay Date: 2015-10-28T03:31:14Z 2288: Pt. 4 Core: Fixed a NONE -> OK_NEW_SCHEMA in ScanBatch.next(). [ScanBatch] (With nearby comments.) commit fcb1438724df83865a83749b097ca19d21cec444 Author: dbarclay Date: 2015-10-28T03:56:33Z 2288: Pt. 4 Hyg.: Edited comments, reordered, whitespace. [ScanBatch] Reordered Added comments. Aligned. commit 9520345aae524d246876a744a8a676e00b0294dc Author: dbarclay Date: 2015-10-28T04:02:25Z 2288: Pt. 4 Core+: Fixed UnionAllRecordBatch to receive IterOutcome sequence right. (3659) [UnionAllRecordBatch] commit 3acf8ec8927974901d825859c8f00d1382aa2d87 Author: dbarclay Date: 2015-10-28T04:05:01Z 2288: Pt. 5 Core: Fixed ScanBatch.Mutator.isNewSchema() to stop spurious "new schema" reports (fix short-circuit OR, to call resetting method right). [ScanBatch] commit 59ede9bda0e73c0bb6840018781f1f404d73f85c Author: dbarclay Date: 2015-10-28T04:11:55Z 2288: Pt. 5 Hyg.: Renamed, edited comments, reordered. [ScanBatch, SchemaChangeCallBack, AbstractSingleRecordBatch] Renamed getSchemaChange -> getSchemaChangedAndReset. Renamed schemaChange -> schemaChanged. Added doc. comments. Aligned. commit d93dc1ee518db409e72b8672679640be65ae7a62 Author: dbarclay Date: 2015-10-28T04:20:47Z 2288: Pt. 6 Core: Avoided dummy Null.IntVec. column in JsonReader when not needed (MapWriter.isEmptyMap()). [JsonReader, 3 vector files] commit f989229ac5510273698af19d87b50044efed81c5 Author: dbarclay Date: 2015-10-28T04:32:44Z 2288: Pt. 6 Hyg.: Edited comments, message. Fixed message formatting. [RecordReader, JSONFormatPlugin, JSONRecordReader, AbstractMapVector, JsonReader] Fixed message formatting. Edited comments. Edited message. Fixed spurious line break. commit a360e9d0d7bfac0aa75831c35585f8ea890858dc Author: dbarclay Date: 2015-10-28T05:06:13Z 2288: Pt. 7 Core: Added column families in HBaseRecordReader* to avoid dummy Null.IntVec. clash. [HBaseRecordReader] commit 18467a9ce14f6328b4766541267183927ebcdc9c Author: dbarclay Date: 2015-10-28T05:06:52Z 2288: Pt. 8 Core.1: Cleared recordCount in OrderedPartitionRecordBatch.innerNext(). [OrderedPartitionRecordBatch] commit 73bf71fc4af8e54599e908abbb993a64b066c097 Author: dbarclay Date: 2015-10-28T05:07:14Z 2288: Pt. 8 Core.2: Cleared recordCount in ProjectRecordBatch.innerNext. [ProjectRecordBatch] commit 064187d1c23f2b4fc09e94066d66e79ef961c4f1 Author: dbarclay Date: 2015-10-28T05:08:22Z 2288: Pt. 8 Core.3: Cleared recordCount in TopNBatch.innerNext. [TopNBatch] commit 8b9d1657ee22cee4432074f778e5d10e2c06e8e8 Author: dbarclay Date: 2015-10-28T05:24:35Z 2288: Pt. 9 Core: Had UnorderedReceiverBatch reset RecordBatchLoader's record count. [UnorderedReceiverBatch, RecordBatchLoader] commit 57c36854d6b65107d46b547fd9edd351c3149643 Author: dbarclay Date: 2015-10-29T00:28:50Z 2288: Pt. 10 Core: W
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/228#discussion_r43660011 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java --- @@ -93,13 +93,15 @@ public void ensureAtLeastOneField(ComplexWriter writer) { if (!atLeastOneWrite) { // if we had no columns, create one empty one so we can return some data for count purposes. SchemaPath sp = columns.get(0); - PathSegment root = sp.getRootSegment(); + PathSegment fieldPath = sp.getRootSegment(); BaseWriter.MapWriter fieldWriter = writer.rootAsMap(); - while (root.getChild() != null && !root.getChild().isArray()) { -fieldWriter = fieldWriter.map(root.getNameSegment().getPath()); -root = root.getChild(); + while (fieldPath.getChild() != null && ! fieldPath.getChild().isArray()) { +fieldWriter = fieldWriter.map(fieldPath.getNameSegment().getPath()); +fieldPath = fieldPath.getChild(); + } + if (fieldWriter.isEmptyMap()) { --- End diff -- That should be just a renaming: It looked like the variable originally named "root" related to the root (from getRootSegment()) only until it was modified in the loop (so the name was misleading from the beginning of the loop and especially at the last reference). Renaming it with "field" was to reflect that it corresponds to variable "fieldWriter" even as both are possibly modified in the loop. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/228#discussion_r43661248 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java --- @@ -189,8 +189,8 @@ protected void putVector(String name, ValueVector vector) { Preconditions.checkNotNull(vector, "vector cannot be null") ); if (old != null && old != vector) { - logger.debug("Field [%s] mutated from [%s] to [%s]", name, old.getClass().getSimpleName(), - vector.getClass().getSimpleName()); + logger.debug("Field [{}] mutated from [{}] to [{}]", name, old.getClass().getSimpleName(), --- End diff -- http://www.slf4j.org/api/org/slf4j/helpers/MessageFormatter.htmlDoesn't the number of brace replacement patterns correspond to the number of arguments? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/228#discussion_r43661592 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java --- @@ -23,60 +23,214 @@ import org.apache.drill.exec.record.selection.SelectionVector4; /** - * A record batch contains a set of field values for a particular range of records. In the case of a record batch - * composed of ValueVectors, ideally a batch fits within L2 cache (~256k per core). The set of value vectors do not - * change unless the next() IterOutcome is a *_NEW_SCHEMA type. - * - * A key thing to know is that the Iterator provided by record batch must align with the rank positions of the field ids - * provided utilizing getValueVectorId(); + * A record batch contains a set of field values for a particular range of --- End diff -- Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/228#discussion_r43664058 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java --- @@ -335,7 +343,8 @@ public IterOutcome nextBatch() throws SchemaChangeException { return iterRight; default: -throw new IllegalStateException(String.format("Unknown state %s.", iterRight)); +throw new IllegalStateException( --- End diff -- Hmm. It's been a while, so I don't recall exactly. It looks like I wrapped it to fit within 80 columns since most of the code and comments just above fit within that width. (Yes, now I see that the next throw statement below is also wider.) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/228#discussion_r43665769 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java --- @@ -122,6 +122,7 @@ protected void killIncoming(final boolean sendUpstream) { @Override public IterOutcome innerNext() { +recordCount = 0; --- End diff -- That wouldn't have worked--the recordCount fields are declared in the subclasses, not in one of those superclasses. (I didn't try refactoring recordCount up into one of those base classes because I was already running into enough downstream problems every time I fixed an upstream increment.) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/228#discussion_r43666158 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java --- @@ -325,10 +325,13 @@ public AggOutcome doWork() { if (EXTRA_DEBUG_1) { logger.debug("Received new schema. Batch has {} records.", incoming.getRecordCount()); } -newSchema = true; -this.cleanup(); -// TODO: new schema case needs to be handled appropriately -return AggOutcome.UPDATE_AGGREGATOR; +final BatchSchema newIncomingSchema = incoming.getSchema(); +if ((! newIncomingSchema.equals(schema)) && schema != null) { --- End diff -- Yeah, I thought this change might not be right. So what do I do (to at least avoid the schema-change exception that I was getting from one of the test suites, but do so without introducing the bug you point out)? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/228#discussion_r43704529 --- Diff: contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseRecordReader.java --- @@ -142,10 +148,12 @@ public void setup(OperatorContext context, OutputMutator output) throws Executio getOrCreateFamilyVector(column.getRootSegment().getPath(), false); } } - logger.debug("Opening scanner for HBase table '{}', Zookeeper quorum '{}', port '{}', znode '{}'.", - hbaseTableName, hbaseConf.get(HConstants.ZOOKEEPER_QUORUM), - hbaseConf.get(HBASE_ZOOKEEPER_PORT), hbaseConf.get(HConstants.ZOOKEEPER_ZNODE_PARENT)); - hTable = new HTable(hbaseConf, hbaseTableName); + // Add vector for any column families not mentioned yet (in order to avoid + // creation of dummy NullableIntVectors for them). + for (HColumnDescriptor columnFamily : --- End diff -- Created DRILL-4010 and added unit test (currently ignored) for it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/228#discussion_r43796603 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java --- @@ -325,10 +325,13 @@ public AggOutcome doWork() { if (EXTRA_DEBUG_1) { logger.debug("Received new schema. Batch has {} records.", incoming.getRecordCount()); } -newSchema = true; -this.cleanup(); -// TODO: new schema case needs to be handled appropriately -return AggOutcome.UPDATE_AGGREGATOR; +final BatchSchema newIncomingSchema = incoming.getSchema(); +if ((! newIncomingSchema.equals(schema)) && schema != null) { --- End diff -- > You shouldn't put a hack in to ignore a valid schema change. Yeah, I'm reverting this HashAggTemplate change. > I don't know all the details but if this is related to HBase, my thought is you probably need > to resolve 4010 to avoid spurious schema changes. It's not clear whether it's related to HBase or not. (The relevant test failure did occur in an HBase test, but it was in a Jenkins-run cluster test that has been hard to reproduce elsewhere. At one point (I thought) I had a reproduction using just JSON files, but now that doesn't seem to work.) > You should make sure to avoid propagating schema changes that are not real/required. Related to that, ExternalSortBatch seems to return two OK_NEW_SCHEMA batches with schemas differing only in their selection vectors: 11:23:50.149 [main] TRACE o.a.d.e.p.i.v.IteratorValidatorBatchIterator - [#3; on ExternalSortBatch]: incoming next() return: #records = 204, schema: BatchSchema [fields=[`key1`(INT:REQUIRED), `alt`(BIGINT:REQUIRED)], selectionVector=FOUR_BYTE], prev. new (not equal): BatchSchema [fields=[`key1`(INT:REQUIRED), `alt`(BIGINT:REQUIRED)], selectionVector=NONE] (The earlier schema ("prev. new:") seems to come from setting up the sort, and the latter one ("schema:") seems to come from completing the sort (it was triggered by a NONE from upstream).) Is that correct? (Should the selection vectors have been the same? Should the second return have been OK instead of OK_NEW_SCHEMA?) If that is correct (e.g., vectors were changed and should have been), how should downstream operators, or at least aggregation, handle that? Should they be able to handle a "technical" schema change (OK_NEW_SCHEMA with possibly-changed vectors or certain details of the schema) that is not a logical schema change (not a change in the logical fields or types in the aggregated value), even if arbitrary logical schema changes can't be handled (e.g., because some don't even make sense)? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/228#discussion_r43797898 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/MapVector.java --- @@ -309,7 +309,14 @@ public Object getObject(int index) { Map vv = new JsonStringHashMap<>(); for (String child:getChildFieldNames()) { ValueVector v = getChild(child); -if (v != null) { +// TODO(DRILL-4001): Resolve this hack: --- End diff -- Added mention of MapVector.getObject(int) and exception message "java.lang.IndexOutOfBoundsException: index: 0, length: 1 (expected: range(0, 0))" to DRILL-4001 report. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/228#discussion_r43808779 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java --- @@ -93,13 +93,15 @@ public void ensureAtLeastOneField(ComplexWriter writer) { if (!atLeastOneWrite) { // if we had no columns, create one empty one so we can return some data for count purposes. SchemaPath sp = columns.get(0); - PathSegment root = sp.getRootSegment(); + PathSegment fieldPath = sp.getRootSegment(); BaseWriter.MapWriter fieldWriter = writer.rootAsMap(); - while (root.getChild() != null && !root.getChild().isArray()) { -fieldWriter = fieldWriter.map(root.getNameSegment().getPath()); -root = root.getChild(); + while (fieldPath.getChild() != null && ! fieldPath.getChild().isArray()) { +fieldWriter = fieldWriter.map(fieldPath.getNameSegment().getPath()); +fieldPath = fieldPath.getChild(); + } + if (fieldWriter.isEmptyMap()) { --- End diff -- Recall that ensureAtLeastOneField used to create that dummy NullableIntVector field/column (so that the schema didn't end up with zero fields/columns) _regardless_ of whether it needed to (i.e., even if the schema already had a column or columns from a previous JsonRecordReader for the same ScanBatch). Under the failure conditions, that blind vector setting frequently replaced a NullableVarCharVector with that NullableIntVector, causing either explicit "schema change not supported" user-targeted errors or internal errors about the mismatch between the NullableIntVector and the expectation of a NullableVarCharVector. The additional check prevents ensureAtLeastOneField from overriding a column in an existing non-empty schema and causing those schema changes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/228#discussion_r43921155 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java --- @@ -325,10 +325,13 @@ public AggOutcome doWork() { if (EXTRA_DEBUG_1) { logger.debug("Received new schema. Batch has {} records.", incoming.getRecordCount()); } -newSchema = true; -this.cleanup(); -// TODO: new schema case needs to be handled appropriately -return AggOutcome.UPDATE_AGGREGATOR; +final BatchSchema newIncomingSchema = incoming.getSchema(); +if ((! newIncomingSchema.equals(schema)) && schema != null) { --- End diff -- Reverted via a13e99b ("2288: Pt. 11 Core Upd.: REVERTED all of bad OK_NEW_SCHEMA schema compâ¦") --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/228#issuecomment-153831379 Note: Something went wrong between me and Git. I intended to address review comments only by adding commits (e.g., rather than by, say, deleting any), in order to avoid obscuring earlier comments here in the GitHub conversation. However, for some reason, more than just the last four update commits (with "Upd.") show up as being added today. I apparently re-ordered some commits during rebasing. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/228#discussion_r43941329 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java --- @@ -93,13 +93,15 @@ public void ensureAtLeastOneField(ComplexWriter writer) { if (!atLeastOneWrite) { // if we had no columns, create one empty one so we can return some data for count purposes. SchemaPath sp = columns.get(0); - PathSegment root = sp.getRootSegment(); + PathSegment fieldPath = sp.getRootSegment(); BaseWriter.MapWriter fieldWriter = writer.rootAsMap(); - while (root.getChild() != null && !root.getChild().isArray()) { -fieldWriter = fieldWriter.map(root.getNameSegment().getPath()); -root = root.getChild(); + while (fieldPath.getChild() != null && ! fieldPath.getChild().isArray()) { +fieldWriter = fieldWriter.map(fieldPath.getNameSegment().getPath()); +fieldPath = fieldPath.getChild(); + } + if (fieldWriter.isEmptyMap()) { --- End diff -- Sorry; I can't yet tell what you mean. (By "the ensureAtLeastOneField operations," did you mean operations _in_ ensureAtLeastOneField (e.g., calling map(...) (in the loop)), the operation of calling ensureAtLeastOneField, or something else? (The first seems to be needed to get to the right map, the second seems to be needed to create a dummy column when one is actually needed.)) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/228#discussion_r43943789 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java --- @@ -93,13 +93,15 @@ public void ensureAtLeastOneField(ComplexWriter writer) { if (!atLeastOneWrite) { // if we had no columns, create one empty one so we can return some data for count purposes. SchemaPath sp = columns.get(0); - PathSegment root = sp.getRootSegment(); + PathSegment fieldPath = sp.getRootSegment(); BaseWriter.MapWriter fieldWriter = writer.rootAsMap(); - while (root.getChild() != null && !root.getChild().isArray()) { -fieldWriter = fieldWriter.map(root.getNameSegment().getPath()); -root = root.getChild(); + while (fieldPath.getChild() != null && ! fieldPath.getChild().isArray()) { +fieldWriter = fieldWriter.map(fieldPath.getNameSegment().getPath()); +fieldPath = fieldPath.getChild(); + } + if (fieldWriter.isEmptyMap()) { --- End diff -- > can we remove the ensureAtLeastOneField property Oh, you mean the atLeastOneWrite property? Yes, I think that will work. I'll test that.. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on a diff in the pull request: https://github.com/apache/drill/pull/228#discussion_r43946374 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java --- @@ -93,13 +93,15 @@ public void ensureAtLeastOneField(ComplexWriter writer) { if (!atLeastOneWrite) { // if we had no columns, create one empty one so we can return some data for count purposes. SchemaPath sp = columns.get(0); - PathSegment root = sp.getRootSegment(); + PathSegment fieldPath = sp.getRootSegment(); BaseWriter.MapWriter fieldWriter = writer.rootAsMap(); - while (root.getChild() != null && !root.getChild().isArray()) { -fieldWriter = fieldWriter.map(root.getNameSegment().getPath()); -root = root.getChild(); + while (fieldPath.getChild() != null && ! fieldPath.getChild().isArray()) { +fieldWriter = fieldWriter.map(fieldPath.getNameSegment().getPath()); +fieldPath = fieldPath.getChild(); + } + if (fieldWriter.isEmptyMap()) { --- End diff -- Another question: > Basically change: > > if (!atLeastOneWrite) { > > to > > if (fieldWriter.isEmptyMap()) { Would that field writer be the writer from writer.rootAsMap(), or does the code still need the loop and its traversal? (It seems that changing the "if (!atLeastOneWrite)" to (the simplified equivalent) of "if (true)" (and keeping the loop) work work, but I don't know what the minimum fix is (i.e., exactly how much of that loop and traversal is needed).) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/228#issuecomment-153918109 Rebased on today's master branch. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2489 and DRILL-2769: JDBC exceptions (& ...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/171#issuecomment-154157257 Rebased on current master. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/228#issuecomment-154281600 Addressed problems revealed by today's rebasing. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
GitHub user dsbos opened a pull request: https://github.com/apache/drill/pull/245 DRILL-2288: Fix ScanBatch violation of IterOutcome protocol and downstream chain of bugs Increments: 2288: Pt. 1 Core: Added unit test. [Drill2288GetColumnsMetadataWhenNoRowsTest, empty.json] 2288: Pt. 1 Core: Changed HBase test table #1's # of regions from 1 to 2. [HBaseTestsSuite] Also added TODO(DRILL-3954) comment about # of regions. 2288: Pt. 2 Core: Documented IterOutcome much more clearly. [RecordBatch] Also edited some related Javadoc. 2288: Pt. 2 Hyg.: Edited doc., added @Override, etc. [AbstractRecordBatch, RecordBatch] Purged unused SetupOutcome. Added @Override. Edited comments. Fix some comments to doc. comments. 2288: Pt. 3 Core&Hyg.: Added validation of IterOutcome sequence. [IteratorValidatorBatchIterator] Also: Renamed internal members for clarity. Added comments. 2288: Pt. 4 Core: Fixed a NONE -> OK_NEW_SCHEMA in ScanBatch.next(). [ScanBatch] (With nearby comments.) 2288: Pt. 4 Hyg.: Edited comments, reordered, whitespace. [ScanBatch] Reordered Added comments. Aligned. 2288: Pt. 4 Core+: Fixed UnionAllRecordBatch to receive IterOutcome sequence right. (3659) [UnionAllRecordBatch] 2288: Pt. 5 Core: Fixed ScanBatch.Mutator.isNewSchema() to stop spurious "new schema" reports (fix short-circuit OR, to call resetting method right). [ScanBatch] 2288: Pt. 5 Hyg.: Renamed, edited comments, reordered. [ScanBatch, SchemaChangeCallBack, AbstractSingleRecordBatch] Renamed getSchemaChange -> getSchemaChangedAndReset. Renamed schemaChange -> schemaChanged. Added doc. comments. Aligned. 2288: Pt. 6 Core: Avoided dummy Null.IntVec. column in JsonReader when not needed (MapWriter.isEmptyMap()). [JsonReader, 3 vector files] 2288: Pt. 6 Hyg.: Edited comments, message. Fixed message formatting. [RecordReader, JSONFormatPlugin, JSONRecordReader, AbstractMapVector, JsonReader] Fixed message formatting. Edited comments. Edited message. Fixed spurious line break. 2288: Pt. 7 Core: Added column families in HBaseRecordReader* to avoid dummy Null.IntVec. clash. [HBaseRecordReader] 2288: Pt. 8 Core.1: Cleared recordCount in OrderedPartitionRecordBatch.innerNext(). [OrderedPartitionRecordBatch] 2288: Pt. 8 Core.2: Cleared recordCount in ProjectRecordBatch.innerNext. [ProjectRecordBatch] 2288: Pt. 8 Core.3: Cleared recordCount in TopNBatch.innerNext. [TopNBatch] 2288: Pt. 9 Core: Had UnorderedReceiverBatch reset RecordBatchLoader's record count. [UnorderedReceiverBatch, RecordBatchLoader] 2288: Pt. 9 Hyg.: Added comments. [RecordBatchLoader] 2288: Pt. 10 Core: Worked around mismatched map child vectors in MapVector.getObject(). [MapVector] 2288: Pt. 11 Core: Added OK_NEW_SCHEMA schema comparison for HashAgg. [HashAggTemplate] 2288: Pt. 12 Core: Fixed memory leak in BaseTestQuery's printing. Fixed bad skipping of RecordBatchLoader.clear(...) and QueryDataBatch.load(...) for zero-row batches in printResult(...). Also, dropped suppression of call to VectorUtil.showVectorAccessibleContent(...) (so zero-row batches are as visible as others). 2288: Pt. 13 Core: Fixed test that used unhandled periods in column alias identifiers. 2288: Misc.: Added # of rows to showVectorAccessibleContent's output. [VectorUtil] 2288: Misc.: Added simple/partial toString() [VectorContainer, AbstractRecordReader, JSONRecordReader, BaseValueVector, FieldSelection, AbstractBaseWriter] 2288: Misc. Hyg.: Added doc. comments to VectorContainer. [VectorContainer] 2288: Misc. Hyg.: Edited comment. [DrillStringUtils] 2288: Misc. Hyg.: Clarified message for unhandled identifier containing period. 2288: Pt. 3 Core&Hyg. Upd.: Added schema comparison result to logging. [IteratorValidatorBatchIterator] 2288: Pt. 7 Core Upd.: Handled HBase columns too re NullableIntVectors. [HBaseRecordReader, TestTableGenerator, TestHBaseFilterPushDown] Created map-child vectors for requested columns. Added unit test method testDummyColumnsAreAvoided, adding new row to test table, updated some row counts. 2288: Pt. 7 Hyg. Upd.: Edited comment. [HBaseRecordReader] 2288: Pt. 11 Core Upd.: REVERTED all of bad OK_NEW_SCHEMA schema comparison for HashAgg. [HashAggTemplate] This reverts commit 0939660f4620c03da97f4e1bf25a27514e6d0b81. 2288: Pt. 6 Core Upd.: Added isEmptyMap override in new (just-rebased-in) PromotableWriter. [PromotableW
[GitHub] drill pull request: DRILL-3848: Increase timeout time on several t...
Github user dsbos closed the pull request at: https://github.com/apache/drill/pull/174 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3641: Doc. RecordBatch.IterOutcome (enum...
Github user dsbos commented on the pull request: https://github.com/apache/drill/pull/113#issuecomment-166020356 Resolved as part of DRILL-2288 patch. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-3641: Doc. RecordBatch.IterOutcome (enum...
Github user dsbos closed the pull request at: https://github.com/apache/drill/pull/113 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos closed the pull request at: https://github.com/apache/drill/pull/245 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2288: Fix ScanBatch violation of IterOut...
Github user dsbos closed the pull request at: https://github.com/apache/drill/pull/228 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request: DRILL-2489 and DRILL-2769: JDBC exceptions (& ...
Github user dsbos closed the pull request at: https://github.com/apache/drill/pull/171 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---