[Tarantool-patches] [PATCH v2 4/6] sql: extend result set with nullability
Nikita Pettik
korablev at tarantool.org
Tue Dec 24 03:26:12 MSK 2019
On 18 Dec 01:29, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
> See 2 comments below.
>
> > diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
> > index 169814a2e..cc5891bb8 100644
> > --- a/src/box/sql/delete.c
> > +++ b/src/box/sql/delete.c
> > @@ -420,6 +420,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
> > sqlVdbeSetNumCols(v, 1);
> > vdbe_metadata_set_col_name(v, 0, "rows deleted");
> > vdbe_metadata_set_col_type(v, 0, "integer");
> > + vdbe_metadata_set_col_nullability(v, 0, -1);
>
> 1. All these set_col_nullability(-1) look really doubtful. You
> don't set collation NULL explicitly, but you set nullability -1
> explicitly. Why?
>
> I think it is much better to fill default values of newly allocated
> sql_column_metadata in sqlVdbeSetNumCols. See my diff (not on the
> branch):
Looks reasonable. Thx, applied.
> ==================================================================
>
> > diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
> > index d3de5770b..92a50dd7b 100644
> > --- a/src/box/sql/vdbeInt.h
> > +++ b/src/box/sql/vdbeInt.h
> > @@ -350,6 +350,11 @@ struct sql_column_metadata {
> > char *name;
> > char *type;
> > char *collation;
> > + /**
> > + * -1 is for any member of result set except for pure
> > + * columns: all other expressions are nullable by default.
> > + */
> > + int8_t nullable : 2;
>
> 2. What is a point of having it :2 bits, if you don't
> have other flags in the same byte? Due to alignment
> this member anyway will become 1 byte.
I guess it's an artifact of one of previous iteration of review fixes
or of the original patch. Anyway, removed this bit field.
More information about the Tarantool-patches
mailing list