Fix OOME with nested derived tables #3411
Conversation
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
| TimeUnit.MILLISECONDS.toNanos(Constants.VIEW_COST_CACHE_MAX_AGE); | ||
|
|
||
| private final TableView view; | ||
| private final QueryExpressionTable table; |
There was a problem hiding this comment.
HidingField: Hiding fields of superclasses may cause confusion and errors. This field is hiding a field of the same name in superclass: Index (details)
(at-me in a reply with help or ignore)
| // then we just compile it again. | ||
| TableView view; | ||
| synchronized (session) { | ||
| view = new TableView(schema, id, cteViewName, querySQL, |
There was a problem hiding this comment.
NULL_DEREFERENCE: object returned by getSchemaWithDefault() could be null and is dereferenced by call to TableView(...) at line 7494.
(at-me in a reply with help or ignore)
|
|
||
| String sql = q.getPlanSQL(DEFAULT_SQL_FLAGS); | ||
| q = (Query) session.prepare(sql, true, true); | ||
| if (!sql.equals(querySQL)) { |
There was a problem hiding this comment.
NULL_DEREFERENCE: object sql last assigned on line 369 could be null and is dereferenced at line 370.
(at-me in a reply with help or ignore)
| // for derived tables we don't need to use LRU because the cache | ||
| // should not grow too large for a single query (we drop the whole | ||
| // cache in this cache is dropped at the end of prepareLocal) | ||
| if (derivedTableIndexCache == null) { |
There was a problem hiding this comment.
THREAD_SAFETY_VIOLATION: Read/Write race. Non-private method SessionLocal.getViewIndexCache(...) reads without synchronization from this.derivedTableIndexCache. Potentially races with write in method SessionLocal.prepareCommand(...).
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me in a reply with help or ignore)
| // should not grow too large for a single query (we drop the whole | ||
| // cache in this cache is dropped at the end of prepareLocal) | ||
| if (derivedTableIndexCache == null) { | ||
| derivedTableIndexCache = new HashMap<>(); |
There was a problem hiding this comment.
THREAD_SAFETY_VIOLATION: Unprotected write. Non-private method SessionLocal.getViewIndexCache(...) writes to field this.derivedTableIndexCache outside of synchronization.
Reporting because another access to the same memory occurs on a background thread, although this access may not.
(at-me in a reply with help or ignore)
1 participant
Views and derived tables now have separate implementations based on the same common implementation.
Immediate recompilation of derived table during initial parsing is removed.
When derived table is recompiled in its index and index conditions are supported, but weren't applied its additional recompilation is removed. I think it shouldn't be performed in all cases, but without it some joins return wrong results. This needs further investigation.
OOME with deeply nested derived tables is fixed. Closes OOME with nested derived tables #3410.