Skip to content
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

[YSQL] Inserting NULL into column with UNIQUE INDEX causes crash #1058

Closed
frozenspider opened this issue Mar 22, 2019 · 2 comments
Closed

[YSQL] Inserting NULL into column with UNIQUE INDEX causes crash #1058

frozenspider opened this issue Mar 22, 2019 · 2 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/disk-format-change This feature will change the on-disk format.

Comments

@frozenspider
Copy link
Contributor

frozenspider commented Mar 22, 2019

psql:

postgres=# CREATE TABLE k.num2 (pk int PRIMARY KEY, i int, vi bigint);
CREATE TABLE
postgres=# CREATE UNIQUE INDEX k_num2_vi ON k.num2 (vi);
CREATE INDEX
postgres=# INSERT INTO k.num2 (pk, i, vi) VALUES (0, 0, NULL);
ERROR:  Timed out: Write(tablet: 9f71927d692747ce9e9eb492d13cb394, num_ops: 1, num_attempts: 343, txn: 85932c01-2bff-4d35-829c-2cf1beed405f) passed its deadline 376402.720s (now: 376402.794s): Network error (yb/util/net/socket.cc:570): Recv() got EOF from remote (error 58)

Server:

F0322 19:03:43.199790 44023808 enums.h:176] Invalid value of enum ValueType (full enum type: yb::docdb::ValueType, expression: type_): 88.
Fatal failure details written to /Users/fs/yugabyte-data/node-1/disk-1/yb-data/tserver/logs/yb-tserver.FATAL.details.2019-03-22T19_03_43.pid28222.txt
F20190322 19:03:43 ../../src/yb/util/enums.h:176] Invalid value of enum ValueType (full enum type: yb::docdb::ValueType, expression: type_): 88.
    @        0x105ca8500  google::LogDestination::LogToSinks()
    @        0x105ca750b  google::LogMessage::SendToLog()
    @        0x105ca7eb5  google::LogMessage::Flush()
    @        0x105cac9bf  google::LogMessageFatal::~LogMessageFatal()
    @        0x105ca8ea9  google::LogMessageFatal::~LogMessageFatal()
    @        0x103f1b70b  yb::FatalInvalidEnumValueInternal<>()
    @        0x103f1bd62  yb::docdb::PrimitiveValue::AppendToKey()
    @        0x103ebca0b  yb::docdb::DocKey::AppendTo()
    @        0x103ebcc30  yb::docdb::DocKey::EncodeAsRefCntPrefix()
    @        0x103f0fd74  yb::docdb::PgsqlWriteOperation::Init()
    @        0x103a71d22  yb::tablet::Tablet::KeyValueBatchFromPgsqlWriteBatch()
    @        0x103a721c4  yb::tablet::Tablet::AcquireLocksAndPerformDocOperations()
    @        0x103aad8c0  yb::tablet::TabletPeer::WriteAsync()
    @        0x1039056f8  yb::tserver::TabletServiceImpl::Write()
    @        0x1049603e7  yb::tserver::TabletServerServiceIf::Handle()
    @        0x105170120  yb::rpc::ServicePoolImpl::Handle()
    @        0x105170c85  yb::rpc::TasksPool<>::WrappedTask::Run()
    @        0x1051788e7  yb::rpc::(anonymous namespace)::Worker::Execute()
    @        0x105809c2f  yb::Thread::SuperviseThread()
    @     0x7fff53850661  _pthread_body
    @     0x7fff5385050d  _pthread_start
    @     0x7fff5384fbf9  thread_start

*** Check failure stack trace: ***
    @        0x105ca77b2  google::LogMessage::SendToLog()
    @        0x105ca7eb5  google::LogMessage::Flush()
    @        0x105cac9bf  google::LogMessageFatal::~LogMessageFatal()
    @        0x105ca8ea9  google::LogMessageFatal::~LogMessageFatal()
    @        0x103f1b70b  yb::FatalInvalidEnumValueInternal<>()
    @        0x103f1bd62  yb::docdb::PrimitiveValue::AppendToKey()
    @        0x103ebca0b  yb::docdb::DocKey::AppendTo()
    @        0x103ebcc30  yb::docdb::DocKey::EncodeAsRefCntPrefix()
    @        0x103f0fd74  yb::docdb::PgsqlWriteOperation::Init()
    @        0x103a71d22  yb::tablet::Tablet::KeyValueBatchFromPgsqlWriteBatch()
    @        0x103a721c4  yb::tablet::Tablet::AcquireLocksAndPerformDocOperations()
    @        0x103aad8c0  yb::tablet::TabletPeer::WriteAsync()
    @        0x1039056f8  yb::tserver::TabletServiceImpl::Write()
    @        0x1049603e7  yb::tserver::TabletServerServiceIf::Handle()
    @        0x105170120  yb::rpc::ServicePoolImpl::Handle()
    @        0x105170c85  yb::rpc::TasksPool<>::WrappedTask::Run()
    @        0x1051788e7  yb::rpc::(anonymous namespace)::Worker::Execute()
    @        0x105809c2f  yb::Thread::SuperviseThread()
    @     0x7fff53850661  _pthread_body
    @     0x7fff5385050d  _pthread_start
    @     0x7fff5384fbf9  thread_start

Process finished with exit code 6

Note that PostgreSQL allows duplicate NULL values for a column with unique index: https://www.postgresql.org/docs/11/ddl-constraints.html#DDL-CONSTRAINTS-UNIQUE-CONSTRAINTS

@frozenspider
Copy link
Contributor Author

frozenspider commented Mar 22, 2019

Note that this is NOT reproducible with YCQL:

cqlsh> CREATE TABLE k.num2 (pk int PRIMARY KEY, i int, vi bigint) WITH transactions = {'enabled': true};
cqlsh> CREATE UNIQUE INDEX k_num2_vi ON k.num2 (vi);
cqlsh> INSERT INTO k.num2 (pk, i, vi) VALUES (0, 0, NULL);
cqlsh> INSERT INTO k.num2 (pk, i, vi) VALUES (1, 0, NULL);
InvalidRequest: Error from server: code=2200 [Invalid query] message="Execution Error. Duplicate value disallowed by unique index k_num2_vi
INSERT INTO k.num2 (pk, i, vi) VALUES (1, 0, NULL);
       ^^^^
 (error -300)"

@frozenspider frozenspider changed the title [YSQL] Inserting NULL value into column with UNIQUE INDEX causes crash [YSQL] Inserting NULL into column with UNIQUE INDEX causes crash Mar 22, 2019
@kmuthukk kmuthukk added ycql area/ysql Yugabyte SQL (YSQL) and removed ycql labels Mar 28, 2019
@frozenspider
Copy link
Contributor Author

This was partially fixes as a part of #945 - NULLs are now inserted normally but aren't exempt from unique constraints (as they should be per SQL standard)

yugabyte-ci pushed a commit that referenced this issue Apr 25, 2019
…EX column

Summary:
Column `ybbasectid` is added to unique index in same way as for non-unique index.
It is used to distinguish rows with NULL value for index column.
In case new value is inserted into index and it is not NULL it is searched by key with `ybbasectid` excluded (which is always the last column, see UniqueIndexSearchKey for details)

Test Plan:
New test cases were added

./yb_build.sh --java-test org.yb.pgsql.TestPgRegressPgMisc

Reviewers: mihnea, robert

Reviewed By: robert

Subscribers: alex, bogdan, bharat, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D6459
yugabyte-ci pushed a commit that referenced this issue Jun 7, 2019
…IQUE INDEX column"

Summary:
Current solution produces problems with data inconsistency (as reported in #1434).

Fix will be reworked. Temporary revert changes of 13c2775 which caused problems

Test Plan:
- run script
```
# Dependencies:
# On CentOS you can install psycopg2 thus:
#
#  sudo yum install postgresql-libs
#  sudo yum install python-psycopg2

import psycopg2;
from multiprocessing.dummy import Pool as ThreadPool

# Two threads inserting unique user-ids, but overlapping user names.
# We have a UNIQUE index on user name. So half of the inserts should
# fail with UniqueConstraintViolation error.
#
# Thread 0 attempts to insert tuples like:
#    user-0-0, name-0
#    user-0-1, name-1
#    user-0-2, name-2
#
# Thread 1 attempts to insert tuples like:
#    user-1-0, name-0
#    user-1-1, name-1
#    user-1-2, name-2
#
# Each thread attempts to insert 1000 rows (so total 2000 attempts).
# But half of those should fail with Unique Contraint Violation
# and we should have 1000 rows finally.
#
# However, we seem to end up sometimes with more rows (like 1001 or
# 1003 etc.)
#
num_users=1000

def create_table():
  conn = psycopg2.connect("host=localhost dbname=postgres user=postgres port=5433")
  conn.set_session(autocommit=True)
  cur = conn.cursor()
  cur.execute("""DROP TABLE IF EXISTS users""");
  cur.execute("""CREATE TABLE IF NOT EXISTS users(
                      id     text,
                      ename  text,
                      age    int,
                      PRIMARY KEY(id))
              """)
  print("Created users table")
  print("====================")
  cur.execute("""
          CREATE UNIQUE INDEX IF NOT EXISTS name_idx ON users(ename)
    """)
  print("Created name_idx on table")

def load_data_slave(thread_num):
  thread_id = str(thread_num)
  conn = psycopg2.connect("host=localhost dbname=postgres user=postgres port=5433")
  conn.set_session(autocommit=True)
  cur = conn.cursor()

  print("Thread-" + thread_id + ": Inserting %d rows..." % (num_users))
  # "name" column is indexed, and may have duplicates across threads.
  num_inserts = 0
  num_uniq_violations = 0
  try:
   for idx in range(num_users):
     try:
       cur.execute("""INSERT INTO users (id, ename, age) VALUES (%s, %s, %s)""",
                   ("user-"+thread_id+"-"+str(idx),
                    "name--"+str(idx),
                     20 + (idx % 50)))
       num_inserts += 1
     except psycopg2.errors.UniqueViolation:
       print("Expected unique constraint violation; thread: " + str(thread_num))
       num_uniq_violations += 1
     except Exception as e:
       print("Expected unique constraint violation; thread: " + str(thread_num))
       num_uniq_violations += 1

  except Exception as e:
    print("Unexpected exception: " + str(e))

  print("Thread-" + thread_id + ": Inserted %d rows" %  (num_inserts))
  print("Thread-" + thread_id + ": Uniq Violations: %d " %  (num_uniq_violations))

def load_data():
  pool = ThreadPool(2)
  results = pool.map(load_data_slave, range(2))

# Main
create_table()
load_data()
```

- check table

```
postgres=# select count(*) from users;
 count
-------
  1000
(1 row)
```

Reviewers: robert, mihnea

Reviewed By: mihnea

Subscribers: kannan, yql, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D6679
@d-uspenskiy d-uspenskiy reopened this Jun 21, 2019
@d-uspenskiy d-uspenskiy added the kind/disk-format-change This feature will change the on-disk format. label Jun 21, 2019
yugabyte-ci pushed a commit that referenced this issue Jul 9, 2019
…ndex table [format change]

Summary:
Caution: New column `ybindexkeysuffix` is added to index table. This change is not backward compatible, upgrade from old version to new one is not possible.

To handle non-unique indexes, `ybbasectid` was added to index key as range column to disambiguate between multiple duplicate values of the index key.
Details are in the 0f7aaaf commit.

To extend same approach on NULL values in columns with unique index new column `ybindexkeysuffix` is introduced in index table.
New column is a range key column and it is added straight before `ybbasectid` column.

Index row of the `N column (key) index` looks like:
```
hash_key1, range_key2, range_key3, ... range_keyN, ybindexkeysuffix, ybbasectid
```

`ybindexkeysuffix` is used to distinguish one key to index row from another in unique index in case at least one key is NULL.
In this case `ybindexkeysuffix` is set to `ybctid` of base table (same value as set to `ybbasectid` column).
In all other case `ybindexkeysuffix` is set to NULL.
So the only case when `ybctid` of base table is stored twice in single index row is when at least one column participated in table's unique index have NULL value.

Also using of `Index Scan` instead of `Seq Scan` in case of `IS NULL/IS NOT NULL` condition is supported now.

Test Plan:
New unit tests were introduced:
- concurrent insert into table with unique secondary index
```
./yb_build.sh release --java-test org.yb.pgsql.TestPgIndex
```
- test for inserts and select NULLs
```
./yb_build.sh release --java-test org.yb.pgsql.TestPgRegressPgMisc
```

Reviewers: mikhail, kannan, neha, mihnea

Reviewed By: neha, mihnea

Subscribers: neil, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D6768
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL) kind/disk-format-change This feature will change the on-disk format.
Projects
None yet
Development

No branches or pull requests

3 participants