Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: Nikita Pettik <korablev@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v2 5/6] sql: extend result set with autoincrement
Date: Tue, 24 Dec 2019 16:30:41 +0100	[thread overview]
Message-ID: <8e77686c-a97a-0781-7534-a4c556222291@tarantool.org> (raw)
In-Reply-To: <20191224002621.GA31231@tarantool.org>

Thanks for the patch!

>> @@ -1845,12 +1846,9 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList,
>>  					space_def->fields[iCol].is_nullable;
>>  				vdbe_metadata_set_col_nullability(v, i,
>>  								  is_nullable);
>> -				if (pTabList->a[j].space->sequence != NULL) {
>> -					int afno =
>> -						pTabList->a[j].space->sequence_fieldno;
>> -					if (afno == iCol)
>> -						vdbe_metadata_set_col_autoincrement(v, i);
>> -				}
>> +				if (space->sequence != NULL &&
>> +				    space->sequence_fieldno == iCol)
>> +					vdbe_metadata_set_col_autoincrement(v, i);

Sorry, I know that I proposed this, but now I am getting a compilation
error here:

tarantool/src/box/sql/select.c:1850:33: error: comparison of integers of different signs: 'uint32_t' (aka 'unsigned int') and 'int' [-Werror,-Wsign-compare]
                                    space->sequence_fieldno == iCol)

I don't know why was not it raised earlier.

I've pushed my review fix on top of this commit. See it below
and on the branch.

================================================================================

commit 53fd4b3b3a27cf99c6c7a71a085574b2fd8a0dc7
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date:   Tue Dec 24 15:56:24 2019 +0100

    Review fix 5/6

diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index ddb2509f4..a19494ed9 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -1847,7 +1847,7 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList,
 				vdbe_metadata_set_col_nullability(v, i,
 								  is_nullable);
 				if (space->sequence != NULL &&
-				    space->sequence_fieldno == iCol)
+				    space->sequence_fieldno == (uint32_t) iCol)
 					vdbe_metadata_set_col_autoincrement(v, i);
 			}
 		} else {

  reply	other threads:[~2019-12-24 15:30 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-11 13:44 [Tarantool-patches] [PATCH v2 0/6] sql: extended metadata Nikita Pettik
     [not found] ` <cover.1576071711.git.korablev@tarantool.org>
2019-12-11 13:44   ` [Tarantool-patches] [PATCH v2 1/6] sql: refactor resulting set metadata Nikita Pettik
2019-12-11 13:44   ` [Tarantool-patches] [PATCH v2 2/6] sql: fix possible null dereference in sql_expr_coll() Nikita Pettik
2019-12-11 13:44   ` [Tarantool-patches] [PATCH v2 3/6] sql: extend result set with collation Nikita Pettik
2019-12-18  0:29     ` Vladislav Shpilevoy
2019-12-24  0:26       ` Nikita Pettik
2019-12-24 15:30         ` Vladislav Shpilevoy
2019-12-11 13:44   ` [Tarantool-patches] [PATCH v2 4/6] sql: extend result set with nullability Nikita Pettik
2019-12-18  0:29     ` Vladislav Shpilevoy
2019-12-24  0:26       ` Nikita Pettik
2019-12-11 13:44   ` [Tarantool-patches] [PATCH v2 5/6] sql: extend result set with autoincrement Nikita Pettik
2019-12-18  0:29     ` Vladislav Shpilevoy
2019-12-24  0:26       ` Nikita Pettik
2019-12-24 15:30         ` Vladislav Shpilevoy [this message]
2019-12-25 12:17           ` Nikita Pettik
2019-12-25 15:40             ` Vladislav Shpilevoy
2019-12-25 23:18               ` Nikita Pettik
2019-12-11 13:44   ` [Tarantool-patches] [PATCH v2 6/6] sql: extend result set with alias Nikita Pettik
2019-12-18  0:29     ` Vladislav Shpilevoy
2019-12-24  0:26       ` Nikita Pettik
2019-12-24 15:34         ` Vladislav Shpilevoy
2019-12-26 11:24           ` Nikita Pettik
2019-12-27 11:53             ` Vladislav Shpilevoy
2019-12-27 23:44             ` Nikita Pettik
2019-12-28  9:29               ` Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8e77686c-a97a-0781-7534-a4c556222291@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2 5/6] sql: extend result set with autoincrement' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox