[Groonga-commit] groonga/groonga at 94a0891 [master] Fix a bug that "int8_column_value <= 100" may return true in --filter

Back to archive index

Kouhei Sutou null+****@clear*****
Tue Oct 21 00:17:38 JST 2014


Kouhei Sutou	2014-10-21 00:17:38 +0900 (Tue, 21 Oct 2014)

  New Revision: 94a0891ebe7243c964907ef6457f3a3aff54a30d
  https://github.com/groonga/groonga/commit/94a0891ebe7243c964907ef6457f3a3aff54a30d

  Message:
    Fix a bug that "int8_column_value <= 100" may return true in --filter
    
    GitHub: fix #219
    
    It is caused by comparison between in signed integer and unsigned
    integer. This change makes all integer literals are treated as signed
    integer (Int32 or Int64).
    
    See added test for reproducible case.

  Added files:
    test/command/suite/select/filter/no_index/int8/less_than_or_equal.expected
    test/command/suite/select/filter/no_index/int8/less_than_or_equal.test
  Modified files:
    lib/expr.c
    test/unit/core/test-expr-parse.c

  Modified: lib/expr.c (+30 -7)
===================================================================
--- lib/expr.c    2014-10-20 22:33:27 +0900 (1730e45)
+++ lib/expr.c    2014-10-21 00:17:38 +0900 (dd70ebe)
@@ -814,6 +814,19 @@ grn_expr_get_var_by_offset(grn_ctx *ctx, grn_obj *expr, unsigned int offset)
   x = code_->value;                                             \
   if (CONSTP(x)) {                                              \
     switch (domain) {                                           \
+    case GRN_DB_INT32:                                          \
+      {                                                         \
+        int value;                                              \
+        value = GRN_INT32_VALUE(x);                             \
+        if (value == (int)0x80000000) {                         \
+          domain = GRN_DB_INT64;                                \
+          x->header.domain = domain;                            \
+          GRN_INT64_SET(ctx, x, -((long long int)value));       \
+        } else {                                                \
+          GRN_INT32_SET(ctx, x, -value);                        \
+        }                                                       \
+      }                                                         \
+      break;                                                    \
     case GRN_DB_UINT32:                                         \
       {                                                         \
         unsigned int value;                                     \
@@ -4366,9 +4379,14 @@ scan_info_build_find_index_column_index(grn_ctx *ctx,
   index = ec->value;
   if (n_rest_codes > 2 &&
       ec[1].value &&
-      ec[1].value->header.domain == GRN_DB_UINT32 &&
+      (ec[1].value->header.domain == GRN_DB_INT32 ||
+       ec[1].value->header.domain == GRN_DB_UINT32) &&
       ec[2].op == GRN_OP_GET_MEMBER) {
-    sid = GRN_UINT32_VALUE(ec[1].value) + 1;
+    if (ec[1].value->header.domain == GRN_DB_INT32) {
+      sid = GRN_INT32_VALUE(ec[1].value) + 1;
+    } else {
+      sid = GRN_UINT32_VALUE(ec[1].value) + 1;
+    }
     offset = 2;
     weight = get_weight(ctx, ec + offset);
   } else {
@@ -6806,7 +6824,7 @@ parse_script(grn_ctx *ctx, efs_info *q)
           rest = rest_float;
         } else {
           const char *rest64 = rest;
-          unsigned int uint32 = grn_atoui(q->cur, q->str_end, &rest);
+          grn_atoui(q->cur, q->str_end, &rest);
           // checks to see grn_atoi failed (see above NOTE)
           if ((int64 > UINT32_MAX) ||
               (q->str_end != rest && *rest >= '0' && *rest <= '9')) {
@@ -6815,11 +6833,16 @@ parse_script(grn_ctx *ctx, efs_info *q)
             GRN_INT64_SET(ctx, &int64buf, int64);
             grn_expr_append_const(ctx, q->e, &int64buf, GRN_OP_PUSH, 1);
             rest = rest64;
+          } else if (int64 > INT32_MAX || int64 < INT32_MIN) {
+            grn_obj int64buf;
+            GRN_INT64_INIT(&int64buf, 0);
+            GRN_INT64_SET(ctx, &int64buf, int64);
+            grn_expr_append_const(ctx, q->e, &int64buf, GRN_OP_PUSH, 1);
           } else {
-            grn_obj uint32buf;
-            GRN_UINT32_INIT(&uint32buf, 0);
-            GRN_UINT32_SET(ctx, &uint32buf, uint32);
-            grn_expr_append_const(ctx, q->e, &uint32buf, GRN_OP_PUSH, 1);
+            grn_obj int32buf;
+            GRN_INT32_INIT(&int32buf, 0);
+            GRN_INT32_SET(ctx, &int32buf, (int32_t)int64);
+            grn_expr_append_const(ctx, q->e, &int32buf, GRN_OP_PUSH, 1);
           }
         }
         PARSE(GRN_EXPR_TOKEN_DECIMAL);

  Added: test/command/suite/select/filter/no_index/int8/less_than_or_equal.expected (+13 -0) 100644
===================================================================
--- /dev/null
+++ test/command/suite/select/filter/no_index/int8/less_than_or_equal.expected    2014-10-21 00:17:38 +0900 (9ae84d7)
@@ -0,0 +1,13 @@
+table_create Numbers TABLE_NO_KEY
+[[0,0.0,0.0],true]
+column_create Numbers number COLUMN_SCALAR Int8
+[[0,0.0,0.0],true]
+load --table Numbers
+[
+{"number": 120},
+{"number": -10},
+{"number": 100}
+]
+[[0,0.0,0.0],3]
+select --table Numbers --filter 'number <= 100'
+[[0,0.0,0.0],[[[2],[["_id","UInt32"],["number","Int8"]],[2,-10],[3,100]]]]

  Added: test/command/suite/select/filter/no_index/int8/less_than_or_equal.test (+11 -0) 100644
===================================================================
--- /dev/null
+++ test/command/suite/select/filter/no_index/int8/less_than_or_equal.test    2014-10-21 00:17:38 +0900 (8114130)
@@ -0,0 +1,11 @@
+table_create Numbers TABLE_NO_KEY
+column_create Numbers number COLUMN_SCALAR Int8
+
+load --table Numbers
+[
+{"number": 120},
+{"number": -10},
+{"number": 100}
+]
+
+select --table Numbers --filter 'number <= 100'

  Modified: test/unit/core/test-expr-parse.c (+6 -6)
===================================================================
--- test/unit/core/test-expr-parse.c    2014-10-20 22:33:27 +0900 (6fdd2db)
+++ test/unit/core/test-expr-parse.c    2014-10-21 00:17:38 +0900 (1987188)
@@ -42,7 +42,7 @@ void test_value_access(void);
 void test_snip(void);
 void test_snip_without_tags(void);
 void test_float_literal(void);
-void test_uint32_literal(void);
+void test_int32_literal(void);
 void test_lager_than_int32_literal(void);
 void test_int64_literal(void);
 void test_long_integer_literal(void);
@@ -811,14 +811,14 @@ test_float_literal(void)
 }
 
 void
-test_uint32_literal(void)
+test_int32_literal(void)
 {
   grn_obj *var;
   const char *str_expr = "var = 123456";
 
   var = parse_numeric_literal(str_expr);
-  cut_assert_equal_int(GRN_DB_UINT32, GRN_OBJ_GET_DOMAIN(var));
-  cut_assert_equal_int(123456, GRN_UINT32_VALUE(var));
+  cut_assert_equal_int(GRN_DB_INT32, GRN_OBJ_GET_DOMAIN(var));
+  cut_assert_equal_int(123456, GRN_INT32_VALUE(var));
 }
 
 void
@@ -828,8 +828,8 @@ test_lager_than_int32_literal(void)
   const char *str_expr = "var = 3456789012";
 
   var = parse_numeric_literal(str_expr);
-  cut_assert_equal_int(GRN_DB_UINT32, GRN_OBJ_GET_DOMAIN(var));
-  cut_assert_equal_int(3456789012U, GRN_UINT32_VALUE(var));
+  cut_assert_equal_int(GRN_DB_INT64, GRN_OBJ_GET_DOMAIN(var));
+  cut_assert_equal_int(G_GINT64_CONSTANT(3456789012), GRN_INT64_VALUE(var));
 }
 
 void
-------------- next part --------------
HTML����������������������������...
Download 



More information about the Groonga-commit mailing list
Back to archive index