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

Allow tables without primary key #39

Closed
hermanlee opened this issue Sep 28, 2015 · 4 comments
Closed

Allow tables without primary key #39

hermanlee opened this issue Sep 28, 2015 · 4 comments

Comments

@hermanlee
Copy link
Contributor

Issue by yoshinorim
Wednesday May 06, 2015 at 00:48 GMT
Originally opened as MySQLOnRocksDB#63


Figure out how realistic it is to create a hidden auto_increment primary key, if not specifying primary key in DDL, instead of returning an error.

@hermanlee
Copy link
Contributor Author

Comment by yoshinorim
Tuesday Sep 22, 2015 at 16:46 GMT


Here are basic design plans. Since some data dictionary format change is needed, it makes some sense to add this feature before GA.

  • DDL
    create a hidden auto_increment internal primary key
  • Dictionary
    Adding a flag of using hidden pk or not
  • Row format
    PK: {key_id, hidden_primary_key, other columns}
    SK: {key_id, sec_key, hidden_primary_key}
  • Utility functions
    has_primary_key()
  • CF
    hidden pk are always stored into default CF
  • Insert
    get next value
    assign hidden pk value (auto_inc)
  • Read
  • memcmp is always done by SK
  • User-Defined unique indexes
    I think we can simply disallow this -- normally it should be easier to convert from unique key to primary key. We don't have any table that has unique key but does not have primary key.
  • RBR slave
    If the table has no PK, the slave will use the first applicable index to find the row to update. It is the same with InnoDB+RBR.

RDBSE_KEYDEF member functions

  • RDBSE_KEYDEF::get_primary_key_tuple()
  • ha_rocksdb::get_row_by_rowid()
  • get by hidden pk
  • ha_rocksdb::convert_record_from_storage_format()
  • RDBSE_KEYDEF::unpack_record()
  • if this table uses hidden pk, exclude the pk from packed_key then unpack.
  • RDBSE_KEYDEF::pack_record()

@hermanlee
Copy link
Contributor Author

Comment by jkedgar
Tuesday Sep 22, 2015 at 17:09 GMT


By using an auto increment key you have to store the last (or next) value somewhere and protect it with a mutex when incrementing it, potentially causing a bottleneck in the code. What about instead of an auto-incrementing column using a 64-bit random value? There would be a very small chance of picking a value that is already in use, but if that occurred you would just get a new value during insert.

I'm not sure what our maximum number of rows in the database is, but even at 100 billion rows, only about 0.000000005% of the possible values would be used making the chance of duplicates still very small.

The pros:

  • no mutex during insert
  • no special storage for next value

The cons:

  • a 64-bit value per row (although this would also be necessary for the auto-increment method if you can't guarantee that the total number of rows inserted is less than about 4 billion).
  • very, very rarely (almost never - but a very small possibility) the insert would fail because of a duplicate value and a new primary key would need to be generated randomly.
  • testing the code that handled a duplicate primary key on insert would need special consideration (i.e. some way of forcing a duplicate value).

@hermanlee
Copy link
Contributor Author

Comment by yoshinorim
Tuesday Sep 22, 2015 at 20:35 GMT


  • Tables without primary keys are relatively rare, and they're not heavily accessed, so improving performance is not much worth spending time.
  • In InnoDB, next auto-inc value is kept in memory. At startup, it just calculates by reading the table -- max(auto_inc)+1. I think we can take same approach in MyRocks. This does not use extra storage. It's less efficient at startup, but would be ok since performance is not much important.

@alxyang
Copy link
Contributor

alxyang commented Feb 4, 2016

closed by https://reviews.facebook.net/D52839

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

No branches or pull requests

3 participants