The bug: A query with a subplan containing a hash join with unnest nodes
on both the build and probe sides would not project the collectionn-typed
slots referenced in unnest nodes of the probe side. The reason is that
we used to first complete the hash join build before opening the probe
side. Since the build does a deep-copy those collection-typed slots
to be unnested in the probe side would not be projected.
Example query that exhibited the bug:
subplan
hash join
nested-loop join
singular row src
unnest t.c1
unnest t.c2
scan t
The tuple of 't' has two-collection typed slots, one for 't.c1', and
another for 't.c2'. If the hash join completes the build without
opening the probe side, then the 't.c2' slot would not be projected
and deep copied into the build-side hash table. That collection
would then be returned in GetNext() of the hash join.
The fix: For hash joins inside a subplan, open child(0) before doing
the build.
Change-Id: I569107b5ecafdbb75f3562707947ecc73951140c
Reviewed-on: http://gerrit.cloudera.org:8080/1128
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
The limit of 100 was determined empirically by generating deeply
nested Parquet and Avro files and then trying to run queries with
and without subplans over them (one absolute table ref vs. all relative
table refs for maximally nested subplans).
Based on those experiments we can handle up to 200 levels of nesting,
but the queries get very slow. At 300 levels, we exceed the stack space
due to the recursive implementation of the scan. Also, we decode the
rep/def levels of Parquet as uint8_t. I settled with 100 because it is
safe, future proof and reasonably high for most practical cases.
Change-Id: Iebdfa96a6dd6060387e38eaedb8ddf0f9901ac24
Reviewed-on: http://gerrit.cloudera.org:8080/905
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
These tests are failing in the old agg/join nightly. This test
references a nested types table, so we need to skip it when the old
agg/join is enabled. So, move it to a place where this already happens.
Change-Id: I09400760fd0b7506e4c127bbef92ab413d5d8615
Reviewed-on: http://gerrit.cloudera.org:8080/1143
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Tested-by: Internal Jenkins
Decimal tables are not generated for all file formats. The
analytic-fns test is run on some of these formats, so fails
when it cannot find the decimal_tiny table. Move the test to
the decimal test that handles this correctly.
Change-Id: Ic23b21ed90496fcc9f2f84cfd3dd92899d00498b
Impala supports reading Parquet files with multiple row groups but
with possible performance degradation due to remote reads. This patch
maximizes scan locality by allowing multiple impalads to scan the
rowgroups in their local splits. Each impalad starts a new scan range
for each split local to it if that split contains row group(s) that
need to be scanned.
Change-Id: Iaecc5fb8e89364780bc59dbfa9ae51d0d124d16e
Reviewed-on: http://gerrit.cloudera.org:8080/908
Reviewed-by: Sailesh Mukil <sailesh@cloudera.com>
Tested-by: Internal Jenkins
The commit:
IMPALA-1621,2241,2271,2330,2352: Lazy switch to IO buffers to reduce min mem needed for PAGG/PHJ
recently lowered a couple of limits from 100m to 40m, which appears to
be too aggressive based on occasionol gvm failures. Let's bump
it back up.
Change-Id: I2c3cc24841cf3a305785890329d77e4e9e74f6e5
Reviewed-on: http://gerrit.cloudera.org:8080/1125
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Tested-by: Internal Jenkins
The bug: During planning, when generating an EmptySetNode for a query
block (or a portion thereof) that contained relative table refs we still
populated the corresponding collection-typed slots in the parent scan,
ultimately hitting a sanity DCHECK in the BE.
The fix: Since those collection-typed slots are never used, the
corresponding parent scan should not materilaize them. When creating
an EmptySetNode we mark the appropriate collection-typed slots
as non-materialized.
Change-Id: If0b9c37c46c0e27be7f1b47c395c8c90b499e323
Reviewed-on: http://gerrit.cloudera.org:8080/1092
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
This patch changes the resolution of overloaded functions so that we
prefer functions where there is no loss of precision in argument types.
Previously, the logic would happily convert DECIMAL to FLOAT even if
there was a more suitable overload available. E.g. greatest(TINYINT,
DECIMAL) was resolved to greatest(FLOAT...) instead of greatest(DECIMAL).
This only changes behaviour when no overload exactly matches the argument
types, but the arguments can be converted with no loss of precision,
e.g. TINYINT to DECIMAL.
This patch introduces a conceptual distinction between strict and
non-strict compatibility. All contexts aside from function matching
use non-strict to support the current behavior of implicitly casting
decimals to floats/doubles.
This patch also makes resolution of overloaded functions consistent
regardless of what order functions were added to the Db - overloads are
checked in a canonical order.
Switching to this canonical order revealed further problems with
overload resolution where the correct overload was selected only because
of the order in which it was added to the database. For example, the
logic equally preferred resolving fn(STRING, TINYINT) to
fn(TIMESTAMP, INT) or fn(STRING, INT). This required changes to the
compatibility matrix.
Various cleanup and simplification of the type compatibility logic is
also included.
Change-Id: I50e657c78cdcb925b616b5b088b801510020e255
Reviewed-on: http://gerrit.cloudera.org:8080/845
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Internal Jenkins
This patch changes the Parquet scanner to check if it can't read the
full footer scan range, indicating that file has been overwritten by a
shorter file without refreshing the table metadata. Before it would
DCHECK. This patch adds a test for this case, as well as the case
where the new file is longer than the metadata states (which fails
with an existing error).
Change-Id: Ie2031ac2dc90e4f2573bd3ca8a3709db60424f07
Reviewed-on: http://gerrit.cloudera.org:8080/1084
Tested-by: Internal Jenkins
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
When a full outer join on the old (non-partitioned)
HashJoinNode, if any join fragment has 0 build rows and 0
probe rows an extra null row will be produced.
Change-Id: I75373edc4f6b3b0c23afba3c1fa363c613f23507
Reviewed-on: http://gerrit.cloudera.org:8080/1068
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Tested-by: Internal Jenkins
The bug was that SubplanNode::GetNext() was not explicitly setting the returned
eos to false if eos had not been reached yet. As a result, a UnionNode with
a SubplanNode as its second operand could return fewer rows than expected becasue
the eos was carried over from the previous union operand, and only a single batch
was returned from the SubplanNode.
The fix is to always set the eos return value in SubplanNode::GetNext().
Change-Id: I9f7516d7b740b9e089ea29b9fe416f3f47314e2c
Reviewed-on: http://gerrit.cloudera.org:8080/1076
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
These tests are failing intermittently for me because the "order by" clause
doesn't yield deterministic results.
Change-Id: I98eec16ec8257611a5b1419ecea401304535f374
Reviewed-on: http://gerrit.cloudera.org:8080/1067
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
The bug was that the analysis of a WITH-clause view containing a
relative table ref would register a collection-typed slot in the parent
tuple descriptor of that relative table ref. The problem is that we use
a dummy analyzer with a fresh global state for the WITH-clause analysis.
Since the descriptor table is part of the global state, it was possible
that we'd register a collection-typed slot with an item tuple id that was
the same as the parent tuple id (or another arbitrary tuple id that has
no meaning in the parent tuple). This parent/item tuple with the same id
lead to an infinite recursion in the backend.
The fix is to not register collection-typed slots in parent tuples when
analyzing a relative table ref inside a WITH-clause view. I added a new
flag to the analyzer to indicate whether it is analyzing a WITH-clause
view.
Change-Id: Ifc1bdebe4577a959aec1f6bb89a73eaa37562458
Reviewed-on: http://gerrit.cloudera.org:8080/1021
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
Fixed trivial errors: query test file was missing a required separator,
memory limit exceeded error message was wrong.
Changed nested types test to use deterministic analytic fn.
Lowered memory limit to get it failing consistently on my machine.
Changed memory limit error message to be less specific, since it can
hit different errors in some cases.
Tested by running:
py.test tests/query_test/test_nested_types.py -k tpch \
--exploration_strategy=exhaustive
Change-Id: Icf301cbeea79b825195e9f60ec250b6e167d41f3
Reviewed-on: http://gerrit.cloudera.org:8080/1029
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Internal Jenkins
It shadows the parse_status_ defined in HdfsScanner, making it so the
Parquet scanner doesn't pick up a bad parse status set in an
HdfsScanner function.
Change-Id: I1e64f1909eaf1af1b1130dda2380db7157a64c2b
Reviewed-on: http://gerrit.cloudera.org:8080/884
Reviewed-by: Skye Wanderman-Milne <skye@cloudera.com>
Tested-by: Internal Jenkins
Move a very expensive semi-join test case to run only on
exhaustive so that it is not run as part of the old hj/agg
jenkins runs where it fails.
Change-Id: I4a0f915e894ceac91d86b366876e47e9cc87255a
Reviewed-on: http://gerrit.cloudera.org:8080/930
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Internal Jenkins
ExecNode::Reset() is not idempotent. It always requires a preceding
call to ExecNode::Open(). The bug was that with nested subplans the
following situation could lead to calling Reset() on the same exec
node multiple times resulting in a DCHECK or crash depending on node
being reset. The scenario is illustrated as follows.
Example Plan:
01:subplan
05:nested-loop join
02:singular row src
04:subplan
...
arbitrary exec node tree
...
03:unnest
00:scan
Sequence of calls leading to double Reset():
1. Node 04 calls Reset() on its child(1)
2. Node 04 returns from GetNext() without calling Open() on its child(1)
3. Node 01 calls Reset() on its child(1)
The problem was that SubplanNode::Reset() of node 04 used to call Reset()
on its child(1) even though child(1) had not been opened yet.
The fix is to only call Reset() on child(1) if it has been opened.
Change-Id: I1d6f5f893c9b773c05dc6cbfe99de9f74e47a888
Reviewed-on: http://gerrit.cloudera.org:8080/916
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
The bug: Several places in the sort assumed that the presence of var-len
slots in the tuple to be sorted implied having var-len blocks.
However, if all var-len slots are NULL or empty, then we can have no
var-len blocks. This case is now properly handled.
Change-Id: Ia3ad3669313e9d494ce2472af7775febfa6f247c
Reviewed-on: http://gerrit.cloudera.org:8080/913
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
In some cases the planner generated plans with rows with no
materialized tuples. Recent changes to the backend caused these to
hit a DCHECK. This patch addresses one case in the planner where it
was possible to create such plans: when the planner generated an
empty node from a select subquery with no from clause. The fix is to
create a materialized tuple based on the select list expressions, in
the same way as we handle these selects when the planner cannot
statically determine they have no result rows.
An example query is included as a test.
It also adds additional checks to the frontend and backend to catch
these invalid rows earlier.
Change-Id: I851f2fb5d389471d0bb764cb85f3c49031a075e4
Reviewed-on: http://gerrit.cloudera.org:8080/911
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Internal Jenkins
In order to consider a subplan at a given point in the plan we require a specific
list of tuple ids and table ref ids to be materialized.
The bug was that our requirement on the table ref ids was not strict enough
to properly prevent re-ordering of subplans across outer/semi joins.
The fix: Just like in the regular join ordering, when considering the placement
of a relative or correlated table ref in the plan, we require that all table refs
to the left of and including any outer/semi join preceding that table ref in the
FROM clause are materialized before adding the plan for that table ref.
Change-Id: If2915c16864fda3e9d5536d5c7d346475c9b9e53
Reviewed-on: http://gerrit.cloudera.org:8080/898
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
This is the first step to fix issues with large memory allocations. In
this patch, the built-in `group_concat` is no longer allowed to allocate
arbitraryly large strings and crash impala, but is limited to the upper
bound of possible allocations in Impala.
This patch does not perform any functional change, but rather avoids
unnecessary crashes. However, it changes the parameter type of
FindChunk() in MemPool to be a signed 64bit integer. This change allows
the mempool to allocate internally memory of more than one 1GB, but the
public interface of Allocate() is not changed, so the general limitation
remains. The reason for this change is as follows:
1) In a UDF FunctionContext::Reallocate() would allocate slightly more
than 512MB from the FreePool.
2) The free pool tries to double this size to alloocate 1GB from the
MemPool.
3) The MemPool doubles the size again and overflows the signed 32bit
integer in the FindChunk() method. This will then only allocate 1GB
instead of the expected 2GB.
What happens is that one of the callers expected a larger allocation
than actually happened, which will in turn lead to memory corruption as
soon as the memory is accessed.
Change-Id: I068835dfa0ac8f7538253d9fa5cfc3fb9d352f6a
Reviewed-on: http://gerrit.cloudera.org:8080/858
Tested-by: Internal Jenkins
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
PAGG and PHJ were using an all-or-nothing approach wrt spilling. In
particular, they were trying to switch to IO-sized buffers for both
streams (aggregated and unaggregated in PAGG; build and probe in PHJ)
of every partition (currently 16 partitions for a total of 32
streams), even if some of the streams had very few rows, they were
empty or simply they would not spill so there was no need to allocate
IO-buffers for them. That was increasing the min mem needed by those
operators in many queries.
This patch decouples the decision to switch to IO-buffers for each
stream of each partition. Streams will switch to IO-sized buffers
whenever the rows they contain do not fit in the first two small
buffers (64KB and 512KB respectively). When we decide to spill a
partition, we switch to IO buffers both streams.
With these change many streams of PAGG and PHJ nodes do not need to
use IO-sized buffers, reducing the min mem requirement. For example,
below is the min mem needed (in MBs) for some of the TPC-H queries.
Some need half or less mem from the mem they needed before:
TPC-H Q3: 645 -> 240
TPC-H Q5: 375 -> 245
TPC-H Q7: 685 -> 265
TPC-H Q8: 740 -> 250
TPC-H Q9: 650 -> 400
TPC-H Q18: 1100 -> 425
TPC-H Q20: 420 -> 250
TPC-H Q21: 975 -> 620
To make this small buffer optimization to work, we had to fix
IMPALA-2352. That is, the AllocateRow() call of
PAGG::ConstructIntermediateTuple() could return unsuccessfully just
because the small buffers of the stream were exhausted. In that case,
previously we would treat it as an indication that there is no memory
left, start spilling a partition and switching all stream to
IO-buffes. Now we make a best effort, trying to first
SwitchToIoffers() and if that is successful, we re-attempt the
AllocateRow() call. See IMPALA-2352 for more details.
Another change is that now SwitchToIoBuffers() will reset the flag
using_small_buffers_ back to false, in case we are in a very low
memory situation and it fails to get a buffer. That allows us to
retry calling SwitchToIoBuffers() once we free up some space. See
IMPALA-2330 for more details.
With the above fixes we should also have fixed IMPALA-2241 and
IMPALA-2271 that are essentially stream::using_small_buffers_-related
DCHECKs.
This patch adds all 22 TPC-H queries in test_mem_usage_scaling test
and updates the per-query min mem limits in it. Additionally, it adds
a new aggregation test that uses the TPC-H dataset for larger
aggregations (TestTPCHAggregationQueries). It also removes some
dead test code.
Change-Id: Ia8ccd0b76f6d37562be21fd4539aedbc2a864d38
Reviewed-on: http://gerrit.cloudera.org:8080/818
Reviewed-by: Ippokratis Pandis <ipandis@cloudera.com>
Tested-by: Internal Jenkins
Conflicts:
tests/query_test/test_aggregation.py
A set of DCHECKs in GetNext() wrongly expected memory to already be transferred. In cases
of queries with limits, the resources most likely will not be transferred. This patch
removes the DCHECKS and transfers the memory at that point if needed.
Change-Id: Ie95ade79db125e0e63c2e45d1b27649b1874d510
Reviewed-on: http://gerrit.cloudera.org:8080/837
Reviewed-by: Sailesh Mukil <sailesh@cloudera.com>
Tested-by: Internal Jenkins
Describe should work if given a path that references to a complex-typed
column of a table. It should produce output that lists the names and
types of all valid subpaths of the column (e.g. struct fields, or
key/val for a map).
This changes some error messages in resolving paths, since we can no
longer definitively determine based on the path length whether the first
path element is meant to be the db or the table.
Change-Id: I8a54e83df67141011ff5396c98f9eb0bde0fb04c
Reviewed-on: http://gerrit.cloudera.org:8080/863
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Internal Jenkins
Our CREATE TABLE LIKE FILE used to put the Parquet schema equivalent
into the comment of each column. Since the Parquet data model is very
verbose for deeply nested types compared to the Impala/Hive type, it
was easy to hit comment length limit in the Hive Metastore.
After this patch we populate the comment with a fixed-size message.
We are still limited by the Hive Metastore constraint on the length
of a type string. This patch adds checks and tests for exceeding
the type and comment length during a CREATE TABLE.
Change-Id: I83518e167cdd56c9e3f99e146269c37dc7b8e22a
Reviewed-on: http://gerrit.cloudera.org:8080/889
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Internal Jenkins
QueryExecState::FetchRowsInternal() doesn't check the query state after evaluating the
select statement expressions with GetRowValue(). These means that, e.g., UDFs that call
SetError() in the select list will not fail the query.
Change-Id: I120d7abbee2a3ed5c5c66ec0a3a9b6e9a6ab10bf
Reviewed-on: http://gerrit.cloudera.org:8080/815
Reviewed-by: Sailesh Mukil <sailesh@cloudera.com>
Tested-by: Internal Jenkins
There was a bug with projecting collection-typed slots in the UnnestNode by setting
them to NULL. The problem was that the same tuple/slot could be referenced by multiple
input rows. As a result, all unnests after the first that operate on the same collection
value would incorrectly return an empty row batch because the slot had been set to NULL
by the first unnesting.
The fix is to ignore the null bit when retrieving a collection-typed slot's value in
the UnnestNode. We still set the null bit after retrieving the value for projection.
This solution purposely ignores the conventional NULL semantics of slots. It is a
temporary hack which must be removed eventually.
We rely on the producer of collection-typed slot values (scan node) to write an empty
array value into such slots when the they are NULL in addition to setting the null bit.
Change-Id: Ie6dc671b3d031f1dfe4d95090b1b6987c2c974da
Reviewed-on: http://gerrit.cloudera.org:8080/859
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
Fixes:
1. Change the planner to not invert null-aware anti join because there is
only a left version. Also, always use a hash join because the
nested-loop join does not support that join mode.
2. Fix PartitionedJoinNode::Reset() and related calls to make the join
usable in subplans with the left null-aware anti join mode.
Change-Id: I8da50747f6a0412c5858fd32b9498f58ed779712
Reviewed-on: http://gerrit.cloudera.org:8080/847
Reviewed-by: Marcel Kornacker <marcel@cloudera.com>
Tested-by: Internal Jenkins
The NLJ node did not follow the expected protocol when need_to_return
is set on a row batch, which means that memory referenced by a rowbatch
can be freed or reused the next time GetNext() is called on the child.
This patch changes the NLJ node to follow the protocol by deep copying
all build side row batches when the need_to_return_ flag is set on the
row batches. This prevents the row batches from referencing memory that
may be freed or reused.
Reenable test that was disabled because of IMPALA-2332 since this was
the root cause.
Change-Id: Idcbb8df12c292b9e2b243e1cef5bdfc1366898d1
Reviewed-on: http://gerrit.cloudera.org:8080/810
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Internal Jenkins
The bug:
There was a MemPool in AnalyticEvalNode with a dual purpose:
(1) Allocate temporary tuples.
(2) Back the FunctionContexts of the aggregate function evaluators.
FunctionContexts use FreePools to do their own memory management using a
pointer-based structure that is stored in the memory blocks themselves.
When calling AnalyticEvalNode::Reset() we reset that mem pool backing
that pointer-based structure. Those pointers were then clobbered by
subsequent allocations (and writes) for temporary tuples, ultimately
resulting in the FreePool incorrectly reporting a double free
while doing a Finalize() of an aggregate function.
The fix:
While there are several other ways to address this issue, I chose to
use a different MemPool for the FunctionContexts because that seemed
to be the most sane and minimally invasive fix. That MemPool is not
reset during AnalyticEvalNode::Reset() because the memory is
ultimately managed by the FreePools of the FunctionContexts.
Change-Id: I42fd60785d3c6dec93436cd9ca64de58d1b15c7e
Reviewed-on: http://gerrit.cloudera.org:8080/857
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
The bug was a simple oversight where copied the array data, but forgot
to update the pointer of the corresponding ArrayValue.
Change-Id: Ib6ec0380f66194efc7ea3eb989535652eb8b526f
Reviewed-on: http://gerrit.cloudera.org:8080/855
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
The dedup logic in row batch serialisation incorrectly assumed that two
distinct tuples must have two distinct memory addresses. This is not
true if one tuple has zero length.
Update the serialisation logic to check for this case and insert a
NULL.
Adds a unit test that exercises this bug prior to the fix and a query
test that also hit a DCHECK prior to the fix.
Change-Id: If163274b3a6c10f8ac6b6bc90eee9ec95830b7dd
Reviewed-on: http://gerrit.cloudera.org:8080/849
Reviewed-by: Marcel Kornacker <marcel@cloudera.com>
Tested-by: Internal Jenkins
This DCHECK condition was overly strict - a non-nullable tuple pointer
can be NULL if the tuple is zero bytes long (since there is no memory
backing the tuple).
Adds a test query that hit the DCHECK.
Change-Id: I16e8bc0db747b83c239de931a4fc9677d5c85ae6
Reviewed-on: http://gerrit.cloudera.org:8080/836
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Internal Jenkins
Tuples are set to NULL for representing non-matches of outer joins.
During planning, the FE identifies at which nodes in the plan which
tuples can be NULL or not. In the BE, codegen uses the tuple nullability
information to remove the runtime NULL checking of tuples.
The bug here was that the tuple nullability information was not preserved
through SubplanNodes, so subsequent nodes could get a SEGV in codegen'd
parts of the execution when dereferencing a NULL tuple because the NULL
check was incorrectly optimized out.
Change-Id: I4356537c0a7153ec1247cc74b6b7952ed9e3d884
Reviewed-on: http://gerrit.cloudera.org:8080/827
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
This patch adds basic end-to-end functional tests for nested types:
1. For exercising the Reset() of exec nodes when inside a subplan.
2. For asserting correct behavior when row batches with collection-typed
slots flow through exec nodes.
Most cases are covered, but there are a few known issues that prevent
full coverage. The remaining tests will be added as part of the fixes
for those existing JIRAs.
Change-Id: I0140c1a32cb5edd189f283c68a24de8484b3f434
Reviewed-on: http://gerrit.cloudera.org:8080/823
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
The hash join and tuple stream code was not handling correctly the
case of joins whose right side had very high cardinality but where
tuple had zero footprint. Any such join with more than 16M tuples
on the right side would crash. In particular, if the tuple footprint
is zero, an infinite number of rows fit in one block. But according to
the old way we were iterating over the rows of the stream, we would
increment by 1 the idx to get the next "row" eventually overflowing
and hitting dcheck.
Another, second, problem was the calculation of the size of the hash
table in such where the footprint of tuples is zero. In such case,
a hash table of minimum size would suffice. Instead we would try to
create a very large hash table to fit the large number of tuples,
resulting to OOM errors.
This patch fixes the two problems by having specific calculation of
the next idx in the stream as well as the size of the hash table in
case the stream contains tuples with zero footprint.
Change-Id: I12469b9c63581fcbc78c87200de7797eac3428c9
Reviewed-on: http://gerrit.cloudera.org:8080/811
Reviewed-by: Ippokratis Pandis <ipandis@cloudera.com>
Tested-by: Internal Jenkins
This patch makes the ownership of the memory backing the tuple pointers of
a RowBatch dependent on whether the legacy joins and aggs are enabled:
By default, the memory is malloc'd and owned by the RowBatch:
If enable_partitioned_hash_join=true and enable_partitioned_aggregation=true
then the memory is owned by the RowBatch and is freed upon its destruction.
This mode is more performant especially with SubplanNodes in the ExecNode tree
because the tuple pointers are not transferred and do not have to be re-created
in every Reset().
Memory is allocated from MemPool:
Otherwise, the memory is allocated from the RowBatch's tuple pool. As a result,
the pointer memory is transferred just like tuple data, and must be re-created
in Reset(). This mode is required for the legacy join and agg which rely on the
tuple pointers being allocated from the RowBatch's tuple pool, so they can
acquire ownership of the tuple pointers.
Performance impact for nested types:
Initial cluster runs and profiling on nested TPCH identified excessive
malloc/frees as a major performance bottleneck. This change paves the way
for further optimizations which yielded a 2x improvement in response time
for most nested TPCH queries.
Change-Id: I4ac58b18058ce46b4db89fbe117b0bcad19e9ee7
Reviewed-on: http://gerrit.cloudera.org:8080/807
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
This caused failures on the non-partitioned agg/join tests and the ASAN
test. Remove the query that failed to unbreak the build. This query can
be readded when IMPALA-2207 is fixed (for the ASAN) test and the cause of
the other failure is diagnosed and fixed.
Change-Id: Idb1ca951e0de05aee3c1237392fff74ddd756ed7
In some cases in the NLJ node eos_ wasn't set even though the limit was
reached. This prevented the limit from being handled correctly before
returning rows to the caller of GetNext(). This could result in either
too many rows being returned, or a crash when the row batch size was
set to an invalid negative number.
The fix is to always check for whether the limit was reached before
returning from GetNext().
Change-Id: I660e774787870213ada9f2d3e6f10953d9937022
Reviewed-on: http://gerrit.cloudera.org:8080/797
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Internal Jenkins
The first attempt in b2b9a10dda942c7e4f2af01be28e819f71de146f was wrong,
but the good news is that the validation checks caught the problem.
Problem with original approach:
In the SubplanNode we used to set the collection-typed slots of the
current row to NULL after the subplan invocation for the current row
was completed. The problem is that we may have already returned rows
from the SubplanNode which still have the non-NULL slots, and some exec
node consumers may have copied the data.
Fixed approach:
We now set a collection-slot to NULL in the UnnestNode that flattens it
immediately after evaluating the corresponding SlotRef, before returning
any rows from the UnnestNode. Setting the slot to NULL as early as possible
ensures that all rows returned by the containing SubplanNode will have the
slot set to NULL.
Change-Id: Ie942d9b2c835589ed9d41c68a831795bbbff2895
Reviewed-on: http://gerrit.cloudera.org:8080/803
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Reviewed-by: Marcel Kornacker <marcel@cloudera.com>
Tested-by: Internal Jenkins
Collection-typed slots are expensive to copy, e.g., during data
exchanges or when writing into a buffered-tuple-stream. Even worse,
such slots could be duplicated many times after unnesting in a
subplan. To alleviate this problem, this patch implements a
poor man's projection where collection-typed slots are set to NULL
inside the SubplanNode that flattens them.
The FE guarantees that the contents of an array-typed slot are never
referenced outside of the single UnnestNode that access them, so when
returning eos in UnnestNode::GetNext() we also set the unnested array
slot to NULL to avoid those expensive copies in downstream exec nodes.
The FE provides that guarantee by creating a new slot in the parent
scan for every relative CollectionTableRef. For example, for a table
't' with a collection-typed column 'c' the following query would have
two separate slots in the tuple of 't', one for 'c1' and one for 'c2':
select * from t, t.c c1, t.c c2
Change-Id: I90e5b86463019c9ed810c299945c831c744ff563
Reviewed-on: http://gerrit.cloudera.org:8080/763
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
We will not provide full nested types support if any of these options
are set:
--enable_partitioned_aggregation=false
--enable_partitioned_hash_join=false
Change-Id: I0f8607914faf9691d5f7b1a4327609fefba22e56
Reviewed-on: http://gerrit.cloudera.org:8080/792
Reviewed-by: Marcel Kornacker <marcel@cloudera.com>
Tested-by: Internal Jenkins
By not using THREAD_LOCAL for its state, btrim() invocations in
multi-threaded contexts (i.e. pushed to the scanner) would have threads
trampling over each other's bitset used to check for trimmed characters.
Testing:
See new test in expr.test:
select count(*) from functional.alltpyes where btrim(string_col, string_col) != ""
.. should give 0 results, but would give > 0 with this bug.
Change-Id: I595e25b1d4fb7c76b846fce837b4ec140f47d43c
Reviewed-on: http://gerrit.cloudera.org:8080/748
Reviewed-by: Henry Robinson <henry@cloudera.com>
Tested-by: Henry Robinson <henry@cloudera.com>
Implement Tuple-to-Tuple DeepCopy for collections. Add query test
that uses the TOP-N node, which deep copies tuples in this way.
Confirmed that the query test failed before this fix.
Change-Id: I3fea860d8251038d7b5eb85c77973939abe9dbf8
Reviewed-on: http://gerrit.cloudera.org:8080/757
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Internal Jenkins
Invalid test file format caused tpch tests to fail.
Change-Id: Ibf523d071bb14db72689e39645fd1724897543c7
Reviewed-on: http://gerrit.cloudera.org:8080/766
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
