TAJO-799: Local query without FROM throws IllegalArgumentException in CLI (Hyoungjun Kim via hyunsik)
Project: http://git-wip-us.apache.org/repos/asf/tajo/repo Commit: http://git-wip-us.apache.org/repos/asf/tajo/commit/0fb823b9 Tree: http://git-wip-us.apache.org/repos/asf/tajo/tree/0fb823b9 Diff: http://git-wip-us.apache.org/repos/asf/tajo/diff/0fb823b9 Branch: refs/heads/window_function Commit: 0fb823b9004a9f566bba55dd40c7defeade946d2 Parents: 6cc5006 Author: Hyunsik Choi <[email protected]> Authored: Wed Apr 30 02:59:05 2014 +0900 Committer: Hyunsik Choi <[email protected]> Committed: Wed Apr 30 02:59:47 2014 +0900 ---------------------------------------------------------------------- CHANGES | 3 + .../java/org/apache/tajo/catalog/TableDesc.java | 2 +- .../apache/tajo/cli/ConnectDatabaseCommand.java | 2 +- .../tajo/cli/DefaultTajoCliOutputFormatter.java | 16 ++-- .../main/java/org/apache/tajo/cli/TajoCli.java | 10 ++- .../java/org/apache/tajo/conf/TajoConf.java | 2 +- .../java/org/apache/tajo/cli/TestTajoCli.java | 78 +++++++++++++++++++- .../testLocalQueryWithoutFrom.result | 8 ++ 8 files changed, 108 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tajo/blob/0fb823b9/CHANGES ---------------------------------------------------------------------- diff --git a/CHANGES b/CHANGES index 4879124..cbfc292 100644 --- a/CHANGES +++ b/CHANGES @@ -19,6 +19,9 @@ Release 0.9.0 - unreleased BUG FIXES + TAJO-799: Local query without FROM throws IllegalArgumentException in + CLI. (Hyoungjun Kim via hyunsik) + TAJO-802: No partition columns in WEB catalog page. (Hyoungjun Kim via hyunsik) http://git-wip-us.apache.org/repos/asf/tajo/blob/0fb823b9/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/TableDesc.java ---------------------------------------------------------------------- diff --git a/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/TableDesc.java b/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/TableDesc.java index 4251116..5aa035e 100644 --- a/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/TableDesc.java +++ b/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/TableDesc.java @@ -73,7 +73,7 @@ public class TableDesc implements ProtoObject<TableDescProto>, GsonObject, Clone public TableDesc(TableDescProto proto) { this(proto.getTableName(), new Schema(proto.getSchema()), - new TableMeta(proto.getMeta()), new Path(proto.getPath()), proto.getIsExternal()); + new TableMeta(proto.getMeta()), proto.hasPath() ? new Path(proto.getPath()) : null, proto.getIsExternal()); if(proto.hasStats()) { this.stats = new TableStats(proto.getStats()); } http://git-wip-us.apache.org/repos/asf/tajo/blob/0fb823b9/tajo-client/src/main/java/org/apache/tajo/cli/ConnectDatabaseCommand.java ---------------------------------------------------------------------- diff --git a/tajo-client/src/main/java/org/apache/tajo/cli/ConnectDatabaseCommand.java b/tajo-client/src/main/java/org/apache/tajo/cli/ConnectDatabaseCommand.java index 78774e5..4ec4611 100644 --- a/tajo-client/src/main/java/org/apache/tajo/cli/ConnectDatabaseCommand.java +++ b/tajo-client/src/main/java/org/apache/tajo/cli/ConnectDatabaseCommand.java @@ -39,7 +39,7 @@ public class ConnectDatabaseCommand extends TajoShellCommand { } else if (cmd.length == 2) { if (!client.existDatabase(cmd[1])) { - context.getOutput().write("No Database Found\n"); + context.getOutput().write(cmd[1] + " database not found\n"); } else { try { if (client.selectDatabase(cmd[1])) { http://git-wip-us.apache.org/repos/asf/tajo/blob/0fb823b9/tajo-client/src/main/java/org/apache/tajo/cli/DefaultTajoCliOutputFormatter.java ---------------------------------------------------------------------- diff --git a/tajo-client/src/main/java/org/apache/tajo/cli/DefaultTajoCliOutputFormatter.java b/tajo-client/src/main/java/org/apache/tajo/cli/DefaultTajoCliOutputFormatter.java index d291414..dd1f911 100644 --- a/tajo-client/src/main/java/org/apache/tajo/cli/DefaultTajoCliOutputFormatter.java +++ b/tajo-client/src/main/java/org/apache/tajo/cli/DefaultTajoCliOutputFormatter.java @@ -53,17 +53,21 @@ public class DefaultTajoCliOutputFormatter implements TajoCliOutputFormatter { private String getQuerySuccessMessage(TableDesc tableDesc, float responseTime, int totalPrintedRows, String postfix) { TableStats stat = tableDesc.getStats(); - String volume = FileUtil.humanReadableByteCount(stat.getNumBytes(), false); - long resultRows = stat.getNumRows(); + String volume = stat == null ? "0 B" : FileUtil.humanReadableByteCount(stat.getNumBytes(), false); + long resultRows = stat == null ? 0 : stat.getNumRows(); long realNumRows = resultRows != 0 ? resultRows : totalPrintedRows; - return "(" + realNumRows + " rows, " + responseTime + " sec, " + volume + " " + postfix + ")"; + return "(" + realNumRows + " rows, " + getResponseTimeReadable(responseTime) + ", " + volume + " " + postfix + ")"; + } + + protected String getResponseTimeReadable(float responseTime) { + return responseTime + " sec"; } @Override public void printResult(PrintWriter sout, InputStream sin, TableDesc tableDesc, float responseTime, ResultSet res) throws Exception { - long resultRows = tableDesc.getStats().getNumRows(); + long resultRows = tableDesc.getStats() == null ? 0 : tableDesc.getStats().getNumRows(); if (resultRows == 0) { resultRows = Integer.MAX_VALUE; } @@ -115,6 +119,7 @@ public class DefaultTajoCliOutputFormatter implements TajoCliOutputFormatter { } } sout.println(getQuerySuccessMessage(tableDesc, responseTime, totalPrintedRows, "selected")); + sout.flush(); } @Override @@ -125,7 +130,8 @@ public class DefaultTajoCliOutputFormatter implements TajoCliOutputFormatter { @Override public void printProgress(PrintWriter sout, QueryStatus status) { sout.println("Progress: " + (int)(status.getProgress() * 100.0f) - + "%, response time: " + ((float)(status.getFinishTime() - status.getSubmitTime()) / 1000.0) + " sec"); + + "%, response time: " + + getResponseTimeReadable((float)((status.getFinishTime() - status.getSubmitTime()) / 1000.0))); sout.flush(); } http://git-wip-us.apache.org/repos/asf/tajo/blob/0fb823b9/tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java ---------------------------------------------------------------------- diff --git a/tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java b/tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java index 9772bf8..959e9df 100644 --- a/tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java +++ b/tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java @@ -125,9 +125,8 @@ public class TajoCli { this.reader = new ConsoleReader(sin, out); this.reader.setExpandEvents(false); this.sout = new PrintWriter(reader.getOutput()); - Class formatterClass = conf.getClass(conf.getVar(ConfVars.CLI__OUTPUT_FORMATTER_CLASS), + Class formatterClass = conf.getClass(ConfVars.CLI_OUTPUT_FORMATTER_CLASS.varname, DefaultTajoCliOutputFormatter.class); - this.outputFormatter = (TajoCliOutputFormatter)formatterClass.newInstance(); this.outputFormatter.init(conf); @@ -477,6 +476,13 @@ public class TajoCli { sout.println("Invalid command " + command +". Try \\? for help."); } + public void close() { + //for testcase + if (client != null) { + client.close(); + } + } + public static void main(String [] args) throws Exception { TajoConf conf = new TajoConf(); TajoCli shell = new TajoCli(conf, args, System.in, System.out); http://git-wip-us.apache.org/repos/asf/tajo/blob/0fb823b9/tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java ---------------------------------------------------------------------- diff --git a/tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java b/tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java index 552f1e4..b7a799d 100644 --- a/tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java +++ b/tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java @@ -249,7 +249,7 @@ public class TajoConf extends Configuration { CLI_PRINT_PAUSE_NUM_RECORDS("tajo.cli.print.pause.num.records", 100), CLI_PRINT_PAUSE("tajo.cli.print.pause", true), CLI_PRINT_ERROR_TRACE("tajo.cli.print.error.trace", true), - CLI__OUTPUT_FORMATTER_CLASS("tajo.cli.otuptu.formatter", "org.apache.tajo.cli.DefaultTajoCliOutputFormatter"); + CLI_OUTPUT_FORMATTER_CLASS("tajo.cli.output.formatter", "org.apache.tajo.cli.DefaultTajoCliOutputFormatter"); ; public final String varname; http://git-wip-us.apache.org/repos/asf/tajo/blob/0fb823b9/tajo-core/src/test/java/org/apache/tajo/cli/TestTajoCli.java ---------------------------------------------------------------------- diff --git a/tajo-core/src/test/java/org/apache/tajo/cli/TestTajoCli.java b/tajo-core/src/test/java/org/apache/tajo/cli/TestTajoCli.java index 02ec15d..799f175 100644 --- a/tajo-core/src/test/java/org/apache/tajo/cli/TestTajoCli.java +++ b/tajo-core/src/test/java/org/apache/tajo/cli/TestTajoCli.java @@ -21,14 +21,56 @@ package org.apache.tajo.cli; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.CommandLineParser; import org.apache.commons.cli.PosixParser; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; import org.apache.tajo.TpchTestBase; import org.apache.tajo.conf.TajoConf; +import org.apache.tajo.conf.TajoConf.ConfVars; +import org.apache.tajo.storage.StorageUtil; +import org.apache.tajo.util.FileUtil; +import org.junit.After; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TestName; + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.net.URL; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; public class TestTajoCli { + protected static final TpchTestBase testBase; + + /** the base path of result directories */ + protected static final Path resultBasePath; + + static { + testBase = TpchTestBase.getInstance(); + URL resultBaseURL = ClassLoader.getSystemResource("results"); + resultBasePath = new Path(resultBaseURL.toString()); + } + + private TajoCli tajoCli; + private Path currentResultPath; + + @Rule + public TestName name = new TestName(); + + public TestTajoCli() { + String className = getClass().getSimpleName(); + currentResultPath = new Path(resultBasePath, className); + } + + @After + public void teadDown() { + if (tajoCli != null) { + tajoCli.close(); + } + } + @Test public void testParseParam() throws Exception { String[] args = new String[]{"-f", "test.sql", "--param", "test1=10", "--param", "test2=20"}; @@ -66,9 +108,9 @@ public class TestTajoCli { TajoConf tajoConf = TpchTestBase.getInstance().getTestingCluster().getConfiguration(); - TajoCli shell = new TajoCli(tajoConf, args, System.in, System.out); - assertEquals("false", shell.getContext().getConf().get("tajo.cli.print.pause")); - assertEquals("256", shell.getContext().getConf().get("tajo.executor.join.inner.in-memory-table-num")); + tajoCli = new TajoCli(tajoConf, args, System.in, System.out); + assertEquals("false", tajoCli.getContext().getConf().get("tajo.cli.print.pause")); + assertEquals("256", tajoCli.getContext().getConf().get("tajo.executor.join.inner.in-memory-table-num")); } @Test @@ -80,4 +122,34 @@ public class TestTajoCli { String expected = "select * from lineitem where l_tax > 10 and l_returnflag > 'A'"; assertEquals(expected, TajoCli.replaceParam(sql, params)); } + + @Test + public void testLocalQueryWithoutFrom() throws Exception { + String sql = "select 'abc', '123'; select substr('123456', 1,3);"; + TajoConf tajoConf = TpchTestBase.getInstance().getTestingCluster().getConfiguration(); + tajoConf.setVar(ConfVars.CLI_OUTPUT_FORMATTER_CLASS, TajoCliOutputTestFormatter.class.getName()); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + tajoCli = new TajoCli(tajoConf, new String[]{}, System.in, out); + tajoCli.executeScript(sql); + String consoleResult = new String(out.toByteArray()); + + assertOutputResult(consoleResult); + } + + private void assertOutputResult(String actual) throws Exception { + String resultFileName = name.getMethodName() + ".result"; + FileSystem fs = currentResultPath.getFileSystem(testBase.getTestingCluster().getConfiguration()); + Path resultFile = StorageUtil.concatPath(currentResultPath, resultFileName); + assertTrue(resultFile.toString() + " existence check", fs.exists(resultFile)); + + String expectedResult = FileUtil.readTextFile(new File(resultFile.toUri())); + assertEquals(expectedResult, actual); + } + + public static class TajoCliOutputTestFormatter extends DefaultTajoCliOutputFormatter { + @Override + protected String getResponseTimeReadable(float responseTime) { + return ""; + } + } } http://git-wip-us.apache.org/repos/asf/tajo/blob/0fb823b9/tajo-core/src/test/resources/results/TestTajoCli/testLocalQueryWithoutFrom.result ---------------------------------------------------------------------- diff --git a/tajo-core/src/test/resources/results/TestTajoCli/testLocalQueryWithoutFrom.result b/tajo-core/src/test/resources/results/TestTajoCli/testLocalQueryWithoutFrom.result new file mode 100644 index 0000000..bf51d16 --- /dev/null +++ b/tajo-core/src/test/resources/results/TestTajoCli/testLocalQueryWithoutFrom.result @@ -0,0 +1,8 @@ +?literal, ?literal_1 +------------------------------- +abc, 123 +(1 rows, , 0 B selected) +?substr +------------------------------- +123 +(1 rows, , 0 B selected)
