Mldb 1675 transpose bound parameter #520
Conversation
sql/execution_pipeline_impl.cc
Outdated
|
|
||
| ColumnName outputName = keep(column.columnName); | ||
| const ExpressionValue & rowContents | ||
| = row.values.at(fieldOffset + ROW_CONTENTS); |
|
|
||
| self.assertEqual(res, expected) | ||
|
|
||
| mldb.run_tests() |
There was a problem hiding this comment.
Donc changer les self.assertEqual en self.assertTableResultEquals là où applicable.
|
J'ai fait une revue atomique. @jeremybarnes est définitivement l'homme de la situation pour réviser l'ensemble de l’œuvre. |
sql/execution_pipeline_impl.cc
Outdated
| @@ -450,7 +525,7 @@ outputAdded() const { | |||
|
|
|||
| SubSelectExecutor:: | |||
| SubSelectExecutor(std::shared_ptr<BoundPipelineElement> boundSelect, | |||
| const BoundParameters & getParam) | |||
There was a problem hiding this comment.
My feeling is that we should make use of either a sparse matrix or tabular dataset for the sub selects; we're getting closer and closer towards reimplementing them anyway.
|
The complexity is rather extreme, on top of an already complex amount of code. I think it would be worth spending some time on the whiteboard, and thinking some more about the interfaces and APIs before jumping into the implementation. My feeling is that the abstractions are't right yet. |
|
So, after wednesday I guess? |
|
Est-ce mort? |
NOT READY FOR MERGE
Im creating the PR to get general comments on the architecture changes, I still have some changes to make before its ready to merge.