Add ResultSetRegistry storage [1/N] #331
Conversation
Signed-off-by: ienkovich <[email protected]>
ienkovich
force-pushed
the
ienkovihc/rs-registry-01
branch
from
774bcce to
a9efe32
Compare
|
|
||
| add_library(ResultSetRegistry ${result_set_registry_source_files}) | ||
|
|
||
| target_link_libraries(ResultSetRegistry SchemaMgr ResultSet StringDictionary DataMgr IR Shared Logger Utils) |
There was a problem hiding this comment.
I see the DataMgr is included in the ResultSetReginstry.h, so it should be a public dependency. Can others be private?
There was a problem hiding this comment.
There are many headers included in the ResultSetRegistry.h which include headers from DataProvider, ResultSet, SchemaMgr, Shared. How do you determine only DataMgr should be public?
There was a problem hiding this comment.
It was simply the first on the list. I didn't mean it has to be the only one. Rather, I wondered if we can hide any.
Signed-off-by: ienkovich <[email protected]>
| CHECK(tables_.count(token.tableId())); | ||
| std::unique_ptr<TableData> table = std::move(tables_.at(token.tableId())); | ||
| mapd_unique_lock<mapd_shared_mutex> table_lock(table->mutex); | ||
| tables_.erase(token.tableId()); |
There was a problem hiding this comment.
Do I understand correctly that you hold data_mutex_ here to prevent map rebalancing? Would it be possible to change the container (to limit pointers invalidation) and narrow the lock scope? I'm not sure it's too important, just a note.
There was a problem hiding this comment.
I don't think it matters much which exact structure we use for tables_. We would still need a lock to read/write this structure. And the fact that this map holds unique_ptr with table data allows us to release the lock once table data is found and a table lock is obtained. So I usually do data_lock.unlock(); when the required table is locked and the scope of this global data lock shouldn't be too big.
Signed-off-by: ienkovich <[email protected]>
This first part introduces ResultSetRegistry to store execution results. We still don't use data and schema providers to access them, which will be introduced in the following patches.
Here is the list of changes:
ResultSetRegistrystorage to hold execution results.RelAlgExecutorputs execution results thereTemporaryTablestructure withResultSetTableandResultSetTableToken. A token is used to access actualResultSetobjects. It also holds database id and table id to later access data and schema through providers. The execution result is removed from the registry when its token is destroyedExecutionResultnow holds a token instead ofResultSetobjectsColumnarResultsis moved to the new library because it is still used to columnarize result data when zero-copy fetch or a simple copy cannot be usedResultSetstructure to have full schema information in it. It will be important later when we enable query execution on result sets