You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by Hyunsik Choi <hy...@apache.org> on 2014/03/01 09:19:45 UTC

Re: Review Request 18632: TAJO-641: NPE in HCatalogStore.addTable().

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18632/#review35898
-----------------------------------------------------------


Thank you for your contribution. I leaved some inline comments. 

In addition, this fix seems to need Jinho's review because he has worked on the related parts. So, I've added Jinho to a specified reviewer.


tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java
<https://reviews.apache.org/r/18632/#comment66712>

    The pronoun 'it' used twice in one statement. It is ambiguous. Could revise this comment?



tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java
<https://reviews.apache.org/r/18632/#comment66710>

    If you extract this part into a method, it would be better.



tajo-client/src/main/java/org/apache/tajo/cli/DescTableCommand.java
<https://reviews.apache.org/r/18632/#comment66711>

    This part may need Jinho's review. He has worked on this part and its related parts.


- Hyunsik Choi


On March 1, 2014, 1:59 a.m., Jung JaeHwa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18632/
> -----------------------------------------------------------
> 
> (Updated March 1, 2014, 1:59 a.m.)
> 
> 
> Review request for Tajo.
> 
> 
> Bugs: TAJO-641
>     https://issues.apache.org/jira/browse/TAJO-641
> 
> 
> Repository: tajo
> 
> 
> Description
> -------
> 
> Creating a table with _csvfile.delimiter_ results in NPE.
> 
> * SQL
> create table table1 (id int, name text, score float, type text) using csv with ('csvfile.delimiter'='|') ;
> 
> * TajoMaster log
> {code:xml}
> 2014-02-28 11:50:26,657 ERROR catalog.CatalogServer (CatalogServer.java:addTable(257)) - 
> java.lang.NullPointerException
> 	at org.apache.tajo.catalog.store.HCatalogStore.addTable(HCatalogStore.java:335)
> 	at org.apache.tajo.catalog.CatalogServer$CatalogProtocolHandler.addTable(CatalogServer.java:255)
> 	at org.apache.tajo.catalog.AbstractCatalogClient$6.call(AbstractCatalogClient.java:161)
> 	at org.apache.tajo.catalog.AbstractCatalogClient$6.call(AbstractCatalogClient.java:158)
> 	at org.apache.tajo.rpc.ServerCallable.withRetries(ServerCallable.java:96)
> 	at org.apache.tajo.catalog.AbstractCatalogClient.addTable(AbstractCatalogClient.java:158)
> 	at org.apache.tajo.master.GlobalEngine.createTableOnPath(GlobalEngine.java:320)
> 	at org.apache.tajo.master.GlobalEngine.createTable(GlobalEngine.java:283)
> 	at org.apache.tajo.master.GlobalEngine.updateQuery(GlobalEngine.java:221)
> 	at org.apache.tajo.master.GlobalEngine.executeQuery(GlobalEngine.java:142)
> 	at org.apache.tajo.master.TajoMasterClientService$TajoMasterClientProtocolServiceHandler.submitQuery(TajoMasterClientService.java:162)
> 	at org.apache.tajo.ipc.TajoMasterClientProtocol$TajoMasterClientProtocolService$2.callBlockingMethod(TajoMasterClientProtocol.java:289)
> 	at org.apache.tajo.rpc.BlockingRpcServer$ServerHandler.messageReceived(BlockingRpcServer.java:103)
> 	at org.jboss.netty.channel.SimpleChannelUpstreamHandler.handleUpstream(SimpleChannelUpstreamHandler.java:70)
> 	at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
> 	at org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendUpstream(DefaultChannelPipeline.java:791)
> 	at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:296)
> 	at org.jboss.netty.handler.codec.oneone.OneToOneDecoder.handleUpstream(OneToOneDecoder.java:70)
> 	at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
> 	at org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendUpstream(DefaultChannelPipeline.java:791)
> 	at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:296)
> 	at org.jboss.netty.handler.codec.frame.FrameDecoder.unfoldAndFireMessageReceived(FrameDecoder.java:462)
> 	at org.jboss.netty.handler.codec.frame.FrameDecoder.callDecode(FrameDecoder.java:443)
> 	at org.jboss.netty.handler.codec.frame.FrameDecoder.messageReceived(FrameDecoder.java:303)
> 	at org.jboss.netty.channel.SimpleChannelUpstreamHandler.handleUpstream(SimpleChannelUpstreamHandler.java:70)
> 	at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
> 	at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:559)
> 	at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:268)
> 	at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:255)
> 	at org.jboss.netty.channel.socket.nio.NioWorker.read(NioWorker.java:88)
> 	at org.jboss.netty.channel.socket.nio.AbstractNioWorker.process(AbstractNioWorker.java:109)
> 	at org.jboss.netty.channel.socket.nio.AbstractNioSelector.run(AbstractNioSelector.java:312)
> 	at org.jboss.netty.channel.socket.nio.AbstractNioWorker.run(AbstractNioWorker.java:90)
> 	at org.jboss.netty.channel.socket.nio.NioWorker.run(NioWorker.java:178)
> 	at org.jboss.netty.util.ThreadRenamingRunnable.run(ThreadRenamingRunnable.java:108)
> 	at org.jboss.netty.util.internal.DeadLockProofWorker$1.run(DeadLockProofWorker.java:42)
> 	at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:895)
> 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:918)
> 	at java.lang.Thread.run(Thread.java:680)
> 
> {code}
> 
> 
> Diffs
> -----
> 
>   tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/Schema.java 725e9c3 
>   tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java 3dc0908 
>   tajo-client/src/main/java/org/apache/tajo/cli/DescTableCommand.java bdeb173 
> 
> Diff: https://reviews.apache.org/r/18632/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jung JaeHwa
> 
>


Re: Review Request 18632: TAJO-641: NPE in HCatalogStore.addTable().

Posted by Jinho Kim <jh...@apache.org>.

> On March 1, 2014, 8:19 a.m., Hyunsik Choi wrote:
> > tajo-client/src/main/java/org/apache/tajo/cli/DescTableCommand.java, lines 89-90
> > <https://reviews.apache.org/r/18632/diff/1/?file=507062#file507062line89>
> >
> >     This part may need Jinho's review. He has worked on this part and its related parts.

escaping only need to the delimiter and the delimiter be escaped already in the SQLAnalyzer

JaeHwa,
Could you add some testcase with specific delimiter in the TestHCatalogStore ?


- Jinho


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18632/#review35898
-----------------------------------------------------------


On March 1, 2014, 8:20 a.m., Jung JaeHwa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18632/
> -----------------------------------------------------------
> 
> (Updated March 1, 2014, 8:20 a.m.)
> 
> 
> Review request for Tajo and Jinho Kim.
> 
> 
> Bugs: TAJO-641
>     https://issues.apache.org/jira/browse/TAJO-641
> 
> 
> Repository: tajo
> 
> 
> Description
> -------
> 
> Creating a table with _csvfile.delimiter_ results in NPE.
> 
> * SQL
> create table table1 (id int, name text, score float, type text) using csv with ('csvfile.delimiter'='|') ;
> 
> * TajoMaster log
> {code:xml}
> 2014-02-28 11:50:26,657 ERROR catalog.CatalogServer (CatalogServer.java:addTable(257)) - 
> java.lang.NullPointerException
> 	at org.apache.tajo.catalog.store.HCatalogStore.addTable(HCatalogStore.java:335)
> 	at org.apache.tajo.catalog.CatalogServer$CatalogProtocolHandler.addTable(CatalogServer.java:255)
> 	at org.apache.tajo.catalog.AbstractCatalogClient$6.call(AbstractCatalogClient.java:161)
> 	at org.apache.tajo.catalog.AbstractCatalogClient$6.call(AbstractCatalogClient.java:158)
> 	at org.apache.tajo.rpc.ServerCallable.withRetries(ServerCallable.java:96)
> 	at org.apache.tajo.catalog.AbstractCatalogClient.addTable(AbstractCatalogClient.java:158)
> 	at org.apache.tajo.master.GlobalEngine.createTableOnPath(GlobalEngine.java:320)
> 	at org.apache.tajo.master.GlobalEngine.createTable(GlobalEngine.java:283)
> 	at org.apache.tajo.master.GlobalEngine.updateQuery(GlobalEngine.java:221)
> 	at org.apache.tajo.master.GlobalEngine.executeQuery(GlobalEngine.java:142)
> 	at org.apache.tajo.master.TajoMasterClientService$TajoMasterClientProtocolServiceHandler.submitQuery(TajoMasterClientService.java:162)
> 	at org.apache.tajo.ipc.TajoMasterClientProtocol$TajoMasterClientProtocolService$2.callBlockingMethod(TajoMasterClientProtocol.java:289)
> 	at org.apache.tajo.rpc.BlockingRpcServer$ServerHandler.messageReceived(BlockingRpcServer.java:103)
> 	at org.jboss.netty.channel.SimpleChannelUpstreamHandler.handleUpstream(SimpleChannelUpstreamHandler.java:70)
> 	at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
> 	at org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendUpstream(DefaultChannelPipeline.java:791)
> 	at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:296)
> 	at org.jboss.netty.handler.codec.oneone.OneToOneDecoder.handleUpstream(OneToOneDecoder.java:70)
> 	at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
> 	at org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendUpstream(DefaultChannelPipeline.java:791)
> 	at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:296)
> 	at org.jboss.netty.handler.codec.frame.FrameDecoder.unfoldAndFireMessageReceived(FrameDecoder.java:462)
> 	at org.jboss.netty.handler.codec.frame.FrameDecoder.callDecode(FrameDecoder.java:443)
> 	at org.jboss.netty.handler.codec.frame.FrameDecoder.messageReceived(FrameDecoder.java:303)
> 	at org.jboss.netty.channel.SimpleChannelUpstreamHandler.handleUpstream(SimpleChannelUpstreamHandler.java:70)
> 	at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
> 	at org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:559)
> 	at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:268)
> 	at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:255)
> 	at org.jboss.netty.channel.socket.nio.NioWorker.read(NioWorker.java:88)
> 	at org.jboss.netty.channel.socket.nio.AbstractNioWorker.process(AbstractNioWorker.java:109)
> 	at org.jboss.netty.channel.socket.nio.AbstractNioSelector.run(AbstractNioSelector.java:312)
> 	at org.jboss.netty.channel.socket.nio.AbstractNioWorker.run(AbstractNioWorker.java:90)
> 	at org.jboss.netty.channel.socket.nio.NioWorker.run(NioWorker.java:178)
> 	at org.jboss.netty.util.ThreadRenamingRunnable.run(ThreadRenamingRunnable.java:108)
> 	at org.jboss.netty.util.internal.DeadLockProofWorker$1.run(DeadLockProofWorker.java:42)
> 	at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:895)
> 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:918)
> 	at java.lang.Thread.run(Thread.java:680)
> 
> {code}
> 
> 
> Diffs
> -----
> 
>   tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/Schema.java 725e9c3 
>   tajo-catalog/tajo-catalog-drivers/tajo-hcatalog/src/main/java/org/apache/tajo/catalog/store/HCatalogStore.java 3dc0908 
>   tajo-client/src/main/java/org/apache/tajo/cli/DescTableCommand.java bdeb173 
> 
> Diff: https://reviews.apache.org/r/18632/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jung JaeHwa
> 
>