New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ordering does not work with more than two iterate statements in one struct #2

Closed
nonrecursive opened this Issue Jan 6, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@nonrecursive

nonrecursive commented Jan 6, 2018

First of all: Thank you for this software! You have just allowed me to use my favorite programming language for web development.

I did encounter a bug in kwebapp though:
If there are two ordered iterate statements in a struct, the order of the first statement is ignored when creating the SQL query.

Example
DB file:

struct test {
  field id int;
  field time epoch;
  field somedata int;

  iterate: order time desc;
  iterate somedata: order time desc;
};

kwebapp-c-source outputs

static  const char *const stmts[STMT__MAX] = {
        /* STMT_TEST_BY_SEARCH_0 */
        "SELECT " DB_SCHEMA_TEST(test) " FROM test",
        /* STMT_TEST_BY_SEARCH_1 */
        "SELECT " DB_SCHEMA_TEST(test) " FROM test WHERE test.somedata = ? ORDER BY test.time DESC",
        /* STMT_TEST_INSERT */
        "INSERT INTO test (id,time,somedata) VALUES (?,?,?)",
};

STMT_TEST_BY_SEARCH_0 should also include ORDER BY test.time DESC.

This bug also occurs when the requested ordering of the two statements is different (one of them is asc, the other is desc).

@kristapsdz

This comment has been minimized.

Show comment
Hide comment
@kristapsdz

kristapsdz Jan 6, 2018

Owner
Owner

kristapsdz commented Jan 6, 2018

kristapsdz pushed a commit that referenced this issue Jan 7, 2018

@kristapsdz

This comment has been minimized.

Show comment
Hide comment
@kristapsdz

kristapsdz Jan 7, 2018

Owner

If the given fix addresses this (it does by my tests), close the issue and I'll get it into a release. Thanks again!

Owner

kristapsdz commented Jan 7, 2018

If the given fix addresses this (it does by my tests), close the issue and I'll get it into a release. Thanks again!

@nonrecursive

This comment has been minimized.

Show comment
Hide comment
@nonrecursive

nonrecursive Jan 10, 2018

While it has fixed my ordering problem (thank you for that!), the fix appears to have introduced a new bug:

In the iterate statements, the string literal in the generated code does not get terminated properly anymore:

Take:

struct test {
  field id int rowid;
  iterate;
};

kwebapp-c-source produces this code:

static	const char *const stmts[STMT__MAX] = {
	/* STMT_TEST_BY_UNIQUE_id */
	"SELECT " DB_SCHEMA_TEST(test) " FROM test WHERE test.id = ?",
	/* STMT_TEST_BY_SEARCH_0 */
	"SELECT " DB_SCHEMA_TEST(test) " FROM test,               /* <-----!!!!!, missing '\"' */
	/* STMT_TEST_INSERT */
	"INSERT INTO test DEFAULT VALUES",
};

nonrecursive commented Jan 10, 2018

While it has fixed my ordering problem (thank you for that!), the fix appears to have introduced a new bug:

In the iterate statements, the string literal in the generated code does not get terminated properly anymore:

Take:

struct test {
  field id int rowid;
  iterate;
};

kwebapp-c-source produces this code:

static	const char *const stmts[STMT__MAX] = {
	/* STMT_TEST_BY_UNIQUE_id */
	"SELECT " DB_SCHEMA_TEST(test) " FROM test WHERE test.id = ?",
	/* STMT_TEST_BY_SEARCH_0 */
	"SELECT " DB_SCHEMA_TEST(test) " FROM test,               /* <-----!!!!!, missing '\"' */
	/* STMT_TEST_INSERT */
	"INSERT INTO test DEFAULT VALUES",
};
@kristapsdz

This comment has been minimized.

Show comment
Hide comment
@kristapsdz

kristapsdz Jan 17, 2018

Owner

My CVS->GitHub script is currently broken, so I can't reflect this here, but I just put up a version on the website that addresses both points. If it works for you, close out the issue! And thanks for taking the time to report.

Owner

kristapsdz commented Jan 17, 2018

My CVS->GitHub script is currently broken, so I can't reflect this here, but I just put up a version on the website that addresses both points. If it works for you, close out the issue! And thanks for taking the time to report.

@nonrecursive

This comment has been minimized.

Show comment
Hide comment
@nonrecursive

nonrecursive Jan 18, 2018

It's working now. Thanks a lot.

nonrecursive commented Jan 18, 2018

It's working now. Thanks a lot.

kristapsdz pushed a commit that referenced this issue Jan 20, 2018

kristapsdz pushed a commit that referenced this issue Jan 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment