[tarantool-patches] Re: [PATCH 1/6] sql: split conflict action and affinity for Expr

[n.pettik]

> On 27 Sep 2018, at 23:24, Vladislav Shpilevoy <v.shpilevoy at tarantool.org> wrote:
> 
> See 1 comment below.
> 
> On 17/09/2018 23:32, Nikita Pettik wrote:
>> From: Georgy Kirichenko <georgy at tarantool.org>
>> Lets introduce separate field in struct Expr to store conflict action of
>> RAISE() function, instead of messing it with affinity.
>> ---
>>  src/box/sql/expr.c      | 46 ++++++++++++++++++----------------------------
>>  src/box/sql/fkey.c      |  2 +-
>>  src/box/sql/parse.y     |  4 ++--
>>  src/box/sql/sqliteInt.h |  5 ++++-
>>  src/box/sql/treeview.c  |  6 ++++--
>>  5 files changed, 29 insertions(+), 34 deletions(-)
>> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
>> index 1d32c9a03..5ca628aac 100644
>> --- a/src/box/sql/sqliteInt.h
>> +++ b/src/box/sql/sqliteInt.h
>> @@ -2106,7 +2106,10 @@ typedef int ynVar;
>>   */
>>  struct Expr {
>>  	u8 op;			/* Operation performed by this node */
>> -	char affinity;		/* The affinity of the column or 0 if not a column */
>> +	/** The affinity of the column or 0 if not a column. */
>> +	enum affinity_type affinity;
>> +	/** Conflict action for RAISE() function. */
>> +	enum on_conflict_action on_conflict_action;
> 
> As I understand, these two attributes are mutually
> exclusive, am I right? Could you put them into a union
> instead of the separate fields?

Yep, surely:

diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 5ca628aac..55d4ae96e 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2106,10 +2106,12 @@ typedef int ynVar;
  */
 struct Expr {
        u8 op;                  /* Operation performed by this node */
-       /** The affinity of the column or 0 if not a column. */
-       enum affinity_type affinity;
-       /** Conflict action for RAISE() function. */
-       enum on_conflict_action on_conflict_action;
+       union {
+               /** The affinity of the column. */
+               enum affinity_type affinity;
+               /** Conflict action for RAISE() function. */
+               enum on_conflict_action on_conflict_action;
+       };