Make InsertTupleIntoChunk more efficient
#416
Conversation
… of the output variables
…x for quick lookup
|
I have two questions:
|
There was a problem hiding this comment.
Sorry for the delay in reviewing this. I left a few comments. Maybe don't spend time on implementing them yet though, because we're currently working on a big change to the reading and filtering. So it's a possibility that we actually end up removing this code.
| @@ -34,6 +34,8 @@ class PostgresScanGlobalState { | |||
| std::vector<duckdb::TableFilter *> m_column_filters; | |||
| /* Duckdb output vector idx with information about postgres column id */ | |||
| duckdb::vector<duckdb::pair<duckdb::idx_t, AttrNumber>> m_output_columns; | |||
| /* Store the column ID which needs to output in the set for quick lookup */ | |||
| duckdb::set<AttrNumber> attr_to_output_set; | |||
| Datum value; | ||
| bool should_free; | ||
| }; | ||
| duckdb::map<idx_t, DetoastedValue> detoasted_values; |
There was a problem hiding this comment.
We don't want to allocate any datastructures in this function, because it's extremely performance sensitive. We should pre-allocate this one like we do for the others. We instead want to allocate them once. Also as described in my other we don't want to use map at all, so let's use a vector for this.
|
Okay, so chances are very high that we'll remove this code completely in 0.3.0. So I don't think it makes sense to continue with this approach. Thanks anyway for your efforts. |
|
For reference #477 is the PR that removes this code. |
Fix #402