[Monetdb-developers] MonetDB: sciql - Be more correct with the "fixed" property of th...

Stefan Manegold Stefan.Manegold at cwi.nl
Wed Mar 7 17:52:44 CET 2012


Jennie,

this checkin appear to trigger different output for the following tests:

sql/test/BugDay_2005-11-09_2.8/Tests/ORDER_BY_evaluation_error.SF-1023658
sql/test/BugTracker-2009/Tests/join_topn.SF-2654133
sql/test/BugTracker-2010/Tests/offset_limited_32bit.SF-2950579
sql/test/BugTracker/Tests/like_exp.SF-1613949
sql/test/bugs/Tests/select_orderby_alias-bug-sf-1024615

Could you please check, whether the new output is the desired and correct
one, and if so, approve it, or otherwise check whether you can fix the code?

No hurry. I just happend to notice this and hence do report it ;-)

Stefan

On Tue, Mar 06, 2012 at 05:42:18PM +0100, Jennie Zhang wrote:
> Changeset: a27b334ae151 for MonetDB
> URL: http://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=a27b334ae151
> Modified Files:
> 	sql/backends/monet5/sql.mx
> 	sql/server/rel_schema.c
> 	sql/server/sql_mvc.c
> 	sql/storage/sql_storage.h
> 	sql/storage/store.c
> 	sql/test/sciql/Tests/01_create_01.sql
> 	sql/test/sciql/Tests/01_create_01.stable.out
> Branch: sciql
> Log Message:
> 
> Be more correct with the "fixed" property of the SQL catalog
> 
> renamed "fixed" into "fixed_array" in the SQL catalog and set it value to FALSE
> (i.s.o. TRUE) for all tables.
> 
> added tests to check if "fixed_array" and "nr_dimensions" in the SQL catalog
> are computed correctly.  apporved new stable.out
> 
> 
> diffs (251 lines):
> 
> diff --git a/sql/backends/monet5/sql.mx b/sql/backends/monet5/sql.mx
> --- a/sql/backends/monet5/sql.mx
> +++ b/sql/backends/monet5/sql.mx
> @@ -1881,7 +1881,7 @@ create_table_or_view( mvc *sql, char *sn
>  		}
>  	}
>  
> -	nt = sql_trans_create_table(sql->session->tr, s, t->base.name, t->query, t->type, t->system, temp, t->commit_action, t->sz, &t->fixed, &t->ndims);
> +	nt = sql_trans_create_table(sql->session->tr, s, t->base.name, t->query, t->type, t->system, temp, t->commit_action, t->sz, t->fixed, t->ndims);
>  
>  	for (n = t->columns.set->h; n; n = n->next) {
>  		sql_column *c = n->data;
> diff --git a/sql/server/rel_schema.c b/sql/server/rel_schema.c
> --- a/sql/server/rel_schema.c
> +++ b/sql/server/rel_schema.c
> @@ -729,7 +729,9 @@ create_column(mvc *sql, symbol *s, sql_s
>  				cs->dim->step = GDKstrdup("");
>  				cs->dim->stop = GDKstrdup("");
>  			}
> -			t->fixed = isFixedDim(cs->dim);
> +			if (!(isFixedDim(cs->dim)))
> +				t->fixed = 0;
> +
>  			/* TODO: the case "ARRAY dim_range_list" is not dealt with */
>  		}
>  		if (column_options(sql, opt_list, ss, t, cs) == SQL_ERR)
> @@ -992,6 +994,8 @@ rel_create_table(mvc *sql, sql_schema *s
>  		int i = 0, j = 0, cnt = 0, *N = NULL, *M = NULL;
>  		lng cntall = 1;
>  
> +		if (isArray(t))
> +			t->fixed = 1;
>  		for (n = columns->h; n; n = n->next) {
>  			symbol *sym = n->data.sym;
>  			int res = table_element(sql, sym, s, t, 0);
> diff --git a/sql/server/sql_mvc.c b/sql/server/sql_mvc.c
> --- a/sql/server/sql_mvc.c
> +++ b/sql/server/sql_mvc.c
> @@ -87,7 +87,7 @@ mvc_init(char *dbname, int debug, store_
>  		mvc_create_column_(m, t, "system", "boolean", 1);
>  		mvc_create_column_(m, t, "commit_action", "smallint", 16);
>  		mvc_create_column_(m, t, "readonly", "boolean", 1);
> -		mvc_create_column_(m, t, "fixed", "boolean", 1);
> +		mvc_create_column_(m, t, "fixed_array", "boolean", 1);
>  		mvc_create_column_(m, t, "nr_dimensions", "int", 32);
>  		mvc_create_column_(m, t, "temporary", "smallint", 16);
>  
> @@ -124,7 +124,7 @@ mvc_init(char *dbname, int debug, store_
>  		mvc_create_column_(m, t, "system", "boolean", 1);
>  		mvc_create_column_(m, t, "commit_action", "smallint", 16);
>  		mvc_create_column_(m, t, "readonly", "boolean", 1);
> -		mvc_create_column_(m, t, "fixed", "boolean", 1);
> +		mvc_create_column_(m, t, "fixed_array", "boolean", 1);
>  		mvc_create_column_(m, t, "nr_dimensions", "int", 32);
>  		mvc_create_column_(m, t, "temporary", "smallint", 16);
>  		if (catalog_version) {
> @@ -1029,13 +1029,13 @@ mvc_create_table(mvc *m, sql_schema *s, 
>  				tt == tt_table? "TABLE":(tt == tt_array? "ARRAY":"OTHER_TT"),
>  				s->base.name, name, tt, system, persistence, commit_action);
>  
> -	/* FIXME: are the NULLs correct values for 'fixed' and 'ndims'? */
>  	if (persistence == SQL_DECLARED_TABLE && (!s || strcmp(s->base.name, dt_schema))) {
>  		/* declared tables should not end up in the catalog */
> -		t = create_sql_table(m->sa, name, tt, system, persistence, commit_action, NULL, NULL);
> +		/* actual values of 'fixed' and 'ndims' are computed later */
> +		t = create_sql_table(m->sa, name, tt, system, persistence, commit_action, 0, 0);
>  		t->s = s;
>  	} else {
> -		t = sql_trans_create_table(m->session->tr, s, name, NULL, tt, system, persistence, commit_action, sz, NULL, NULL);
> +		t = sql_trans_create_table(m->session->tr, s, name, NULL, tt, system, persistence, commit_action, sz, 0, 0);
>  	}
>  	return t;
>  }
> @@ -1049,11 +1049,11 @@ mvc_create_view(mvc *m, sql_schema *s, c
>  		fprintf(stderr, "#mvc_create_view %s %s %s\n", s->base.name, name, sql);
>  
>  	if (persistence == SQL_DECLARED_TABLE) {
> -		t = create_sql_table(m->sa, name, tt_view, system, persistence, 0, NULL, NULL);
> +		t = create_sql_table(m->sa, name, tt_view, system, persistence, 0, 0, 0);
>  		t->s = s;
>  		t->query = sa_strdup(m->sa, sql);
>  	} else {
> -		t = sql_trans_create_table(m->session->tr, s, name, sql, tt_view, system, SQL_PERSIST, 0, 0, NULL, NULL);
> +		t = sql_trans_create_table(m->session->tr, s, name, sql, tt_view, system, SQL_PERSIST, 0, 0, 0, 0);
>  	}
>  	return t;
>  }
> @@ -1067,11 +1067,11 @@ mvc_create_remote(mvc *m, sql_schema *s,
>  		fprintf(stderr, "#mvc_create_remote %s %s %s\n", s->base.name, name, loc);
>  
>  	if (persistence == SQL_DECLARED_TABLE) {
> -		t = create_sql_table(m->sa, name, tt_remote, 0, persistence, 0, NULL, NULL);
> +		t = create_sql_table(m->sa, name, tt_remote, 0, persistence, 0, 0, 0);
>  		t->s = s;
>  		t->query = sa_strdup(m->sa, loc);
>  	} else {
> -		t = sql_trans_create_table(m->session->tr, s, name, loc, tt_remote, 0, SQL_REMOTE, 0, 0, NULL, NULL);
> +		t = sql_trans_create_table(m->session->tr, s, name, loc, tt_remote, 0, SQL_REMOTE, 0, 0, 0, 0);
>  	}
>  	return t;
>  }
> @@ -1084,7 +1084,7 @@ mvc_create_generated(mvc *m, sql_schema 
>  	if (mvc_debug)
>  		fprintf(stderr, "#mvc_create_generated %s %s %s\n", s->base.name, name, sql);
>  
> -	t = sql_trans_create_table(m->session->tr, s, name, sql, tt_generated, system, SQL_PERSIST, 0, 0, NULL, NULL);
> +	t = sql_trans_create_table(m->session->tr, s, name, sql, tt_generated, system, SQL_PERSIST, 0, 0, 0, 0);
>  	return t;
>  }
>  
> diff --git a/sql/storage/sql_storage.h b/sql/storage/sql_storage.h
> --- a/sql/storage/sql_storage.h
> +++ b/sql/storage/sql_storage.h
> @@ -314,7 +314,7 @@ extern void reset_functions(sql_trans *t
>  extern sql_schema *sql_trans_create_schema(sql_trans *tr, char *name, int auth_id, int owner);
>  extern void sql_trans_drop_schema(sql_trans *tr, int id, int drop_action);
>  
> -extern sql_table *sql_trans_create_table(sql_trans *tr, sql_schema *s, char *name, char *sql, int tt, bit system, int persistence, int commit_action, int sz, bit *fixed, int *ndims);
> +extern sql_table *sql_trans_create_table(sql_trans *tr, sql_schema *s, char *name, char *sql, int tt, bit system, int persistence, int commit_action, int sz, bit fixed, int ndims);
>  extern sql_table *sql_trans_add_table(sql_trans *tr, sql_table *mt, sql_table *pt);
>  extern sql_table *sql_trans_del_table(sql_trans *tr, sql_table *mt, sql_table *pt, int drop_action);
>  
> @@ -368,7 +368,7 @@ extern int sql_trans_connect_catalog(sql
>  extern int sql_trans_disconnect_catalog(sql_trans *tr, char *db_alias);
>  extern int sql_trans_disconnect_catalog_ALL(sql_trans *tr);
>  
> -extern sql_table *create_sql_table(sql_allocator *sa, char *name, sht type, bit system, int persistence, int commit_action, bit *fixed, int *ndims);
> +extern sql_table *create_sql_table(sql_allocator *sa, char *name, sht type, bit system, int persistence, int commit_action, bit fixed, int ndims);
>  extern sql_column *create_sql_column(sql_allocator *sa, sql_table *t, char *name, sql_subtype *tpe);
>  extern sql_ukey *create_sql_ukey(sql_allocator *sa, sql_table *t, char *nme, key_type kt);
>  extern sql_fkey *create_sql_fkey(sql_allocator *sa, sql_table *t, char *nme, key_type kt, sql_key *rkey, int on_delete, int on_update );
> diff --git a/sql/storage/store.c b/sql/storage/store.c
> --- a/sql/storage/store.c
> +++ b/sql/storage/store.c
> @@ -578,7 +578,7 @@ load_table(sql_trans *tr, sql_schema *s,
>  	t->cleared = 0;
>  	v = table_funcs.column_find_value(tr, find_sql_column(tables, "readonly"),rid);
>  	t->readonly = *(bit *)v;	_DELETE(v);
> -	v = table_funcs.column_find_value(tr, find_sql_column(tables, "fixed"),rid);
> +	v = table_funcs.column_find_value(tr, find_sql_column(tables, "fixed_array"),rid);
>  	t->fixed = *(bit *)v;	_DELETE(v);
>  	v = table_funcs.column_find_value(tr, find_sql_column(tables, "nr_dimensions"),rid);
>  	t->ndims = *(int *)v;	_DELETE(v);
> @@ -1127,7 +1127,7 @@ bootstrap_create_column(sql_trans *tr, s
>  }
>  
>  sql_table *
> -create_sql_table(sql_allocator *sa, char *name, sht type, bit system, int persistence, int commit_action, bit *fixed, int *ndims)
> +create_sql_table(sql_allocator *sa, char *name, sht type, bit system, int persistence, int commit_action, bit fixed, int ndims)
>  {
>  	sql_table *t = SA_ZNEW(sa, sql_table);
>  
> @@ -1149,9 +1149,9 @@ create_sql_table(sql_allocator *sa, char
>  	t->pkey = NULL;
>  	t->sz = COLSIZE;
>  	t->cleared = 0;
> -	/* 'fixed' and 'ndims' are ONLY used for arrays.  Pass NULLs to get their defaults. */
> -	t->fixed = fixed ? *fixed : 1; /* should be set to 0 if an unbounded dimension is found. */
> -	t->ndims = ndims ? *ndims : 0;
> +	/* 'fixed' and 'ndims' are ONLY used for arrays. For tables, their values arre always 0. */
> +	t->fixed = isArray(t) ? fixed : 0; /* should be set to 1 for fixed arrays. */
> +	t->ndims = isArray(t) ? ndims : 0;
>  	t->s = NULL;
>  	return t;
>  }
> @@ -1188,7 +1188,7 @@ sql_table *
>  dup_sql_table(sql_allocator *sa, sql_table *t)
>  {
>  	node *n;
> -	sql_table *nt = create_sql_table(sa, t->base.name, t->type, t->system, SQL_DECLARED_TABLE, t->commit_action, &t->fixed, &t->ndims);
> +	sql_table *nt = create_sql_table(sa, t->base.name, t->type, t->system, SQL_DECLARED_TABLE, t->commit_action, t->fixed, t->ndims);
>  
>  	for (n = t->columns.set->h; n; n = n->next) 
>  		dup_sql_column(sa, nt, n->data);
> @@ -1223,7 +1223,7 @@ bootstrap_create_table(sql_trans *tr, sq
>  	int istmp = isTempSchema(s);
>  	int persistence = istmp?SQL_GLOBAL_TEMP:SQL_PERSIST;
>  	sht commit_action = istmp?CA_PRESERVE:CA_COMMIT;
> -	sql_table *t = create_sql_table(tr->sa, name, tt_table, 1, persistence, commit_action, NULL, NULL /* we never have arrays here */);
> +	sql_table *t = create_sql_table(tr->sa, name, tt_table, 1, persistence, commit_action, 0, 0 /* we never have arrays here */);
>  
>  	if (bs_debug)
>  		fprintf(stderr, "#bootstrap_create_table %s\n", name );
> @@ -1411,7 +1411,7 @@ store_init(int debug, store_type store, 
>  		bootstrap_create_column(tr, t, "system", "boolean", 1);
>  		bootstrap_create_column(tr, t, "commit_action", "smallint", 16);
>  		bootstrap_create_column(tr, t, "readonly", "boolean", 1);
> -		bootstrap_create_column(tr, t, "fixed", "boolean", 1);
> +		bootstrap_create_column(tr, t, "fixed_array", "boolean", 1);
>  		bootstrap_create_column(tr, t, "nr_dimensions", "int", 32);
>  
>  		t = bootstrap_create_table(tr, s, "_columns");
> @@ -3863,7 +3863,7 @@ sql_trans_del_table(sql_trans *tr, sql_t
>  }
>  
>  sql_table *
> -sql_trans_create_table(sql_trans *tr, sql_schema *s, char *name, char *sql, int tt, bit system, int persistence, int commit_action, int sz, bit *fixed, int *ndims)
> +sql_trans_create_table(sql_trans *tr, sql_schema *s, char *name, char *sql, int tt, bit system, int persistence, int commit_action, int sz, bit fixed, int ndims)
>  {
>  	sql_table *t = create_sql_table(tr->sa, name, tt, system, persistence, commit_action, fixed, ndims);
>  	sql_schema *syss = find_sql_schema(tr, isGlobal(t)?"sys":"tmp");
> diff --git a/sql/test/sciql/Tests/01_create_01.sql b/sql/test/sciql/Tests/01_create_01.sql
> --- a/sql/test/sciql/Tests/01_create_01.sql
> +++ b/sql/test/sciql/Tests/01_create_01.sql
> @@ -1,5 +1,16 @@
>  -- use the [size] shortcut for integer type dimensions
>  CREATE ARRAY ary(x TINYINT DIMENSION[4], y BIGINT DIMENSION[-5], v FLOAT DEFAULT 3.7);
>  SELECT * FROM ary;
> +
> +CREATE ARRAY ary1(x TINYINT DIMENSION, y BIGINT DIMENSION, v FLOAT DEFAULT 3.7);
> +SELECT * FROM ary1;
> +
> +CREATE ARRAY ary2(x TINYINT DIMENSION[4], y BIGINT DIMENSION, z BIGINT DIMENSION[-5], v FLOAT DEFAULT 3.7);
> +SELECT * FROM ary2;
> +
> +SELECT * FROM _tables WHERE type = 7 OR fixed_array = TRUE;
> +
>  DROP ARRAY ary;
> +DROP ARRAY ary1;
> +DROP ARRAY ary2;
>  
> diff --git a/sql/test/sciql/Tests/01_create_01.stable.out b/sql/test/sciql/Tests/01_create_01.stable.out
> --- a/sql/test/sciql/Tests/01_create_01.stable.out
> +++ b/sql/test/sciql/Tests/01_create_01.stable.out
> @@ -49,6 +49,26 @@ Ready.
>  [ 3,	-2,	3.7	]
>  [ 3,	-3,	3.7	]
>  [ 3,	-4,	3.7	]
> +#CREATE ARRAY ary1(x TINYINT DIMENSION, y BIGINT DIMENSION, v FLOAT DEFAULT 3.7);
> +#SELECT * FROM ary1;
> +% sys.ary1,	sys.ary1,	sys.ary1 # table_name
> +% x,	y,	v # name
> +% tinyint,	bigint,	double # type
> +% 1,	1,	24 # length
> +#CREATE ARRAY ary2(x TINYINT DIMENSION[4], y BIGINT DIMENSION, z BIGINT DIMENSION[-5], v FLOAT DEFAULT 3.7);
> +#SELECT * FROM ary2;
> +% sys.ary2,	sys.ary2,	sys.ary2,	sys.ary2 # table_name
> +% x,	y,	z,	v # name
> +% tinyint,	bigint,	bigint,	double # type
> +% 1,	1,	1,	24 # length
> +#SELECT * FROM _tables WHERE type = 7 OR fixed_array = TRUE;
> +% sys._tables,	sys._tables,	sys._tables,	sys._tables,	sys._tables,	sys._tables,	sys._tables,	sys._tables,	sys._tables,	sys._tables # table_name
> +% id,	name,	schema_id,	query,	type,	system,	commit_action,	readonly,	fixed_array,	nr_dimensions # name
> +% int,	varchar,	int,	varchar,	smallint,	boolean,	smallint,	boolean,	boolean,	int # type
> +% 4,	4,	4,	0,	1,	5,	1,	5,	5,	1 # length
> +[ 6461,	"ary",	2000,	NULL,	7,	false,	0,	false,	true,	2	]
> +[ 6466,	"ary1",	2000,	NULL,	7,	false,	0,	false,	false,	2	]
> +[ 6472,	"ary2",	2000,	NULL,	7,	false,	0,	false,	false,	3	]
>  #DROP ARRAY ary;
>  
>  # 13:50:46 >  
> _______________________________________________
> Checkin-list mailing list
> Checkin-list at monetdb.org
> http://mail.monetdb.org/mailman/listinfo/checkin-list

-- 
| Stefan.Manegold @ CWI.nl | DB Architectures (INS1) |
| http://CWI.nl/~manegold/ | Science Park 123 (L321) |
| Tel.: +31 (0)20 592-4212 | 1098 XG Amsterdam  (NL) |




More information about the developers-list mailing list