From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <tarantool-patches-bounce@freelists.org>
Received: from localhost (localhost [127.0.0.1])
	by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 87DBF2A793
	for <tarantool-patches@freelists.org>; Mon, 17 Sep 2018 19:06:10 -0400 (EDT)
Received: from turing.freelists.org ([127.0.0.1])
	by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id atZd863UpvHu for <tarantool-patches@freelists.org>;
	Mon, 17 Sep 2018 19:06:10 -0400 (EDT)
Received: from smtp42.i.mail.ru (smtp42.i.mail.ru [94.100.177.102])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id CB6E52A776
	for <tarantool-patches@freelists.org>; Mon, 17 Sep 2018 19:06:09 -0400 (EDT)
Date: Tue, 18 Sep 2018 02:06:08 +0300
From: Alexander Turenko <alexander.turenko@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH] jdbc: fix NPE in update
Message-ID: <20180917230608.q7li5cy2s2xaywtz@tkn_work_nb>
References: <1537196954-23804-1-git-send-email-ztarvos@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <1537196954-23804-1-git-send-email-ztarvos@gmail.com>
Sender: tarantool-patches-bounce@freelists.org
Errors-to: tarantool-patches-bounce@freelists.org
Reply-To: tarantool-patches@freelists.org
List-help: <mailto:ecartis@freelists.org?Subject=help>
List-unsubscribe: <tarantool-patches-request@freelists.org?Subject=unsubscribe>
List-software: Ecartis version 1.0.0
List-Id: tarantool-patches <tarantool-patches.freelists.org>
List-subscribe: <tarantool-patches-request@freelists.org?Subject=subscribe>
List-owner: <mailto:>
List-post: <mailto:tarantool-patches@freelists.org>
List-archive: <http://www.freelists.org/archives/tarantool-patches>
To: Sergei Kalashnikov <ztarvos@gmail.com>
Cc: tarantool-patches@freelists.org

Hi Sergei!

The patch looks good for me in general. Minor comments are below.

WBR, Alexander Turenko.

On Mon, Sep 17, 2018 at 06:09:14PM +0300, Sergei Kalashnikov wrote:
> Updated constants to be in sync with tarantool IPROTO.
> Added integration tests for the working JDBC functionality.
> Fixed travis script to use tarantool 2.0 which is required for SQL.
> Fixed failing user permission calls in tarantool initialization script.
> 
> Closes #39
> ---
> https://github.com/tarantool/tarantool-java/issues/39
> https://github.com/ztarvos/tarantool-java/commits/ztarvos/gh-39-fix-update-npe
> 
> diff --git a/.travis.yml b/.travis.yml
> index e0f434f..852807c 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -11,5 +11,5 @@ before_script:
>    - src/test/travis.pre.sh
>  
>  script:
> -  - mvn test
> +  - mvn verify

Maybe we should run both unit and integration tests. The master barnch
now have one unit test so maybe it worth to leave it here to don't
forget during future rebase on master.

> +            "INSERT INTO test_types VALUES(" +
> +                    "1," +
> +                    "'abcd'," + //CHAR
> +                    "'000000000000000000001'," + //VARCHAR
> +                    "'0000000000000000000000000000000001'," + //LONGVARCHAR
> +                    "100," + // NUMERIC
> +                    "100.1," + // DECIMAL
> +                    "1," + //BIT
> +                    "7," + //TINYINT
> +                    "1000," + //SMALLINT
> +                    "100," + //INTEGER
> +                    "100000000000000000," + //BIGINT
> +                    "-100.2," + //REAL
> +                    "100.3," + //FLOAT
> +                    "X'01020304'," + //BINARY
> +                    "X'0102030405'," +//VARBINARY
> +                    "X'010203040506'," + //LONGVARBINARY
> +                    "'1983-03-14'," + //DATE
> +                    "'12:01:06'," + //TIME
> +                    "129479994)" //TIMESTAMP
> +    };
> +

Can we also store expected values for
JdbcResultSetIT.{testGetColumnByIdx,testGetColumnByName}() somewhere
within this class to improve code locality?