* [tarantool-patches] [PATCH v1 1/1] sql: fix assertion on invalid PK column name
@ 2018-07-12 6:30 Kirill Shcherbatov
2018-07-12 8:54 ` [tarantool-patches] " Vladislav Shpilevoy
2018-07-13 6:54 ` Kirill Yukhin
0 siblings, 2 replies; 5+ messages in thread
From: Kirill Shcherbatov @ 2018-07-12 6:30 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy, Kirill Shcherbatov
Close #3522.
---
https://github.com/tarantool/tarantool/compare/kshch/gh-3522-invalid-primary-key-column-name
https://github.com/tarantool/tarantool/issues/3522
src/box/sql/build.c | 1 -
test/sql/gh-2929-primary-key.result | 7 +++++++
test/sql/gh-2929-primary-key.test.lua | 5 +++++
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index e1d896e..5584d83 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -964,7 +964,6 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */
}
}
}
- assert(pCol == &pTab->aCol[iCol]);
if (nTerm == 1 && pCol != NULL &&
(pTab->def->fields[iCol].type == FIELD_TYPE_INTEGER) &&
sortOrder != SORT_ORDER_DESC) {
diff --git a/test/sql/gh-2929-primary-key.result b/test/sql/gh-2929-primary-key.result
index c238ed7..66a9b96 100644
--- a/test/sql/gh-2929-primary-key.result
+++ b/test/sql/gh-2929-primary-key.result
@@ -35,3 +35,10 @@ box.sql.execute("CREATE TABLE t5(a, b UNIQUE)")
box.sql.execute("DROP TABLE t1")
---
...
+--
+-- gh-3522: invalid primary key name
+--
+box.sql.execute("CREATE TABLE tx (a INT, PRIMARY KEY (b));")
+---
+- error: 'no such column: B'
+...
diff --git a/test/sql/gh-2929-primary-key.test.lua b/test/sql/gh-2929-primary-key.test.lua
index 0a9395c..0e05354 100644
--- a/test/sql/gh-2929-primary-key.test.lua
+++ b/test/sql/gh-2929-primary-key.test.lua
@@ -16,3 +16,8 @@ box.sql.execute("CREATE TABLE t4(a, b)")
box.sql.execute("CREATE TABLE t5(a, b UNIQUE)")
box.sql.execute("DROP TABLE t1")
+
+--
+-- gh-3522: invalid primary key name
+--
+box.sql.execute("CREATE TABLE tx (a INT, PRIMARY KEY (b));")
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread* [tarantool-patches] Re: [PATCH v1 1/1] sql: fix assertion on invalid PK column name
2018-07-12 6:30 [tarantool-patches] [PATCH v1 1/1] sql: fix assertion on invalid PK column name Kirill Shcherbatov
@ 2018-07-12 8:54 ` Vladislav Shpilevoy
2018-07-12 10:03 ` Kirill Shcherbatov
2018-07-13 6:54 ` Kirill Yukhin
1 sibling, 1 reply; 5+ messages in thread
From: Vladislav Shpilevoy @ 2018-07-12 8:54 UTC (permalink / raw)
To: Kirill Shcherbatov, tarantool-patches
Hello. Thanks for the patch!
I know, that removal of assertion always 'fixes' it, but please,
either fix it without removal (and for me it is not obvious why it
should be removed), or explain, why it is invalid.
On 12/07/2018 09:30, Kirill Shcherbatov wrote:
> Close #3522.
> ---
> https://github.com/tarantool/tarantool/compare/kshch/gh-3522-invalid-primary-key-column-name
> https://github.com/tarantool/tarantool/issues/3522
>
> src/box/sql/build.c | 1 -
> test/sql/gh-2929-primary-key.result | 7 +++++++
> test/sql/gh-2929-primary-key.test.lua | 5 +++++
> 3 files changed, 12 insertions(+), 1 deletion(-)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: fix assertion on invalid PK column name
2018-07-12 8:54 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2018-07-12 10:03 ` Kirill Shcherbatov
2018-07-12 10:28 ` Vladislav Shpilevoy
0 siblings, 1 reply; 5+ messages in thread
From: Kirill Shcherbatov @ 2018-07-12 10:03 UTC (permalink / raw)
To: tarantool-patches; +Cc: Vladislav Shpilevoy
> I know, that removal of assertion always 'fixes' it, but please,
> either fix it without removal (and for me it is not obvious why it
> should be removed), or explain, why it is invalid.
I've included this assertion during refactoring and there is no root causes to
keep it, but as we verbally discuss, let's make it working correctly appending
additional pCol == NULL OR condition that code below correctly deal with.
========================================
If specified PK column modifier refers nonexistent column,
lookup by name keeps NULL column pointer unchanged that should
be accounted in assert.
Close #3522.
---
https://github.com/tarantool/tarantool/compare/kshch/gh-3522-invalid-primary-key-column-name
https://github.com/tarantool/tarantool/issues/3522
src/box/sql/build.c | 2 +-
test/sql/gh-2929-primary-key.result | 7 +++++++
test/sql/gh-2929-primary-key.test.lua | 5 +++++
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index e1d896e..5d04e3e 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -964,7 +964,7 @@ sqlite3AddPrimaryKey(Parse * pParse, /* Parsing context */
}
}
}
- assert(pCol == &pTab->aCol[iCol]);
+ assert(pCol == NULL || pCol == &pTab->aCol[iCol]);
if (nTerm == 1 && pCol != NULL &&
(pTab->def->fields[iCol].type == FIELD_TYPE_INTEGER) &&
sortOrder != SORT_ORDER_DESC) {
diff --git a/test/sql/gh-2929-primary-key.result b/test/sql/gh-2929-primary-key.result
index c238ed7..66a9b96 100644
--- a/test/sql/gh-2929-primary-key.result
+++ b/test/sql/gh-2929-primary-key.result
@@ -35,3 +35,10 @@ box.sql.execute("CREATE TABLE t5(a, b UNIQUE)")
box.sql.execute("DROP TABLE t1")
---
...
+--
+-- gh-3522: invalid primary key name
+--
+box.sql.execute("CREATE TABLE tx (a INT, PRIMARY KEY (b));")
+---
+- error: 'no such column: B'
+...
diff --git a/test/sql/gh-2929-primary-key.test.lua b/test/sql/gh-2929-primary-key.test.lua
index 0a9395c..0e05354 100644
--- a/test/sql/gh-2929-primary-key.test.lua
+++ b/test/sql/gh-2929-primary-key.test.lua
@@ -16,3 +16,8 @@ box.sql.execute("CREATE TABLE t4(a, b)")
box.sql.execute("CREATE TABLE t5(a, b UNIQUE)")
box.sql.execute("DROP TABLE t1")
+
+--
+-- gh-3522: invalid primary key name
+--
+box.sql.execute("CREATE TABLE tx (a INT, PRIMARY KEY (b));")
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread* [tarantool-patches] Re: [PATCH v1 1/1] sql: fix assertion on invalid PK column name
2018-07-12 10:03 ` Kirill Shcherbatov
@ 2018-07-12 10:28 ` Vladislav Shpilevoy
0 siblings, 0 replies; 5+ messages in thread
From: Vladislav Shpilevoy @ 2018-07-12 10:28 UTC (permalink / raw)
To: Kirill Shcherbatov, tarantool-patches
LGTM.
On 12/07/2018 13:03, Kirill Shcherbatov wrote:
>> I know, that removal of assertion always 'fixes' it, but please,
>> either fix it without removal (and for me it is not obvious why it
>> should be removed), or explain, why it is invalid.
> I've included this assertion during refactoring and there is no root causes to
> keep it, but as we verbally discuss, let's make it working correctly appending
> additional pCol == NULL OR condition that code below correctly deal with.
>
> ========================================
>
> If specified PK column modifier refers nonexistent column,
> lookup by name keeps NULL column pointer unchanged that should
> be accounted in assert.
>
> Close #3522.
> ---
> https://github.com/tarantool/tarantool/compare/kshch/gh-3522-invalid-primary-key-column-name
> https://github.com/tarantool/tarantool/issues/3522
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: fix assertion on invalid PK column name
2018-07-12 6:30 [tarantool-patches] [PATCH v1 1/1] sql: fix assertion on invalid PK column name Kirill Shcherbatov
2018-07-12 8:54 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2018-07-13 6:54 ` Kirill Yukhin
1 sibling, 0 replies; 5+ messages in thread
From: Kirill Yukhin @ 2018-07-13 6:54 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy, Kirill Shcherbatov
Hello,
On 12 июл 09:30, Kirill Shcherbatov wrote:
> Close #3522.
> ---
> https://github.com/tarantool/tarantool/compare/kshch/gh-3522-invalid-primary-key-column-name
> https://github.com/tarantool/tarantool/issues/3522
I've checked the patch into 2.0 branch.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-07-13 6:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-12 6:30 [tarantool-patches] [PATCH v1 1/1] sql: fix assertion on invalid PK column name Kirill Shcherbatov
2018-07-12 8:54 ` [tarantool-patches] " Vladislav Shpilevoy
2018-07-12 10:03 ` Kirill Shcherbatov
2018-07-12 10:28 ` Vladislav Shpilevoy
2018-07-13 6:54 ` Kirill Yukhin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox