Avoid JIT imports in BodoSQL C++ backend#907
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #907 +/- ##
==========================================
+ Coverage 66.68% 68.80% +2.11%
==========================================
Files 186 195 +9
Lines 66795 67547 +752
Branches 9507 9593 +86
==========================================
+ Hits 44543 46473 +1930
+ Misses 19572 18245 -1327
- Partials 2680 2829 +149 |
| from urllib.parse import parse_qsl, urlparse | ||
|
|
||
|
|
||
| def parse_dbtype(con_str) -> tuple[str, str]: |
There was a problem hiding this comment.
The code in this file is just moved here and not changed.
DrTodd13
left a comment
DrTodd13
left a comment
There was a problem hiding this comment.
Might be helpful on this type of PR to note when code that is moved is also changed. Also, might help to describe categories of changes and reasoning so you can judge at that level once rather than try to figure out the reason for 50 identical changes. Anyway, everything looks reasonable to me.
| Unsupported = 29 | ||
|
|
||
|
|
||
| # Scalar dtypes for supported Bodo Arrays |
There was a problem hiding this comment.
Moved compiler related code to context_ext.py and added a no-JIT path for getting SQL data types from dataframes and TablePaths.
| def _ensure_dynamic_params_list(dynamic_params_list: Any) -> list: | ||
| """Verify the supplied Dynamic params list is a supported type | ||
| and converts the result to a list. | ||
| def add_table_type( |
There was a problem hiding this comment.
Just moved here with minor changes to avoid importing JIT for typing unnecessarily.
| JavaEntryPoint.addTableToSchema(schema, table) | ||
|
|
||
|
|
||
| def _generate_table_read( |
There was a problem hiding this comment.
Just moved here with minor changes to avoid importing JIT for typing unnecessarily.
| return read_line | ||
|
|
||
|
|
||
| def create_java_dynamic_parameter_type_list(dynamic_params_list: list[Any]): |
There was a problem hiding this comment.
Moved here with changes to avoid importing JIT for typing unnecessarily.
| return build_java_array_list(types_list) | ||
|
|
||
|
|
||
| def create_java_named_parameter_type_map(named_params: dict[str, Any]): |
There was a problem hiding this comment.
Moved here with changes to avoid importing JIT for typing unnecessarily.
| ) | ||
|
|
||
|
|
||
| # Scalar dtypes for supported Bodo Arrays |
There was a problem hiding this comment.
All this code is just moved here.
Thanks. Makes sense. Added some comments for the next reviewer. |
scott-routledge2
left a comment
scott-routledge2
left a comment
There was a problem hiding this comment.
LGTM, thanks @ehsantn !
3 participants
Changes included in this PR
Splits JIT and non-JIT code in BodoSQL to allow lazy JIT import (if necessary) in the new C++ backend.
Testing strategy
Opened an issue to check for avoiding JIT imports in tests since needs some work.
User facing changes
None.
Checklist
[run CI]in your commit message.