A recent change, IMPALA-5016, added an expr rewrite rule to simplfy
coalesce(). This rule eliminates the coalesce() when its first
parameter (that isn't constant null) is a SlotRef pointing to a
SlotDescriptor that is non-nullable (for example because it is from
a non-nullable Kudu column or because it is from an HDFS partition
column with no null partitions), under the assumption that the SlotRef
could never have a null value.
This assumption is violated when the SlotRef is the output of an
outer join, leading to incorrect results being returned. The problem
is that the nullability of a SlotDescriptor (which determines whether
there is a null indicator bit in the tuple for that slot) is a
slightly different property than the nullability of a SlotRef pointing
to that SlotDescriptor (since the SlotRef can still be NULL if the
entire tuple is NULL).
This patch removes the portion of the rewrite rule that considers
the nullability of the SlotDescriptor. This means that we're missing
out on some optimizations opportunities and we should revisit this in
a way that works with outer joins (IMPALA-5753)
Testing:
- Updated FE tests.
- Added regression tests to exprs.test
Change-Id: I1ca6df949f9d416ab207016236dbcb5886295337
Reviewed-on: http://gerrit.cloudera.org:8080/7567
Reviewed-by: Matthew Jacobs <mj@cloudera.com>
Reviewed-by: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Tested-by: Impala Public Jenkins
This is intended to be merged at the same time as Part 2 but is
separated out to make the change more reviewable. Part 2 assumes
that it does not need special logic to handle this mode (e.g.
because the old aggs and joins don't use reservation).
Disable the --enable_partitioned_{aggregation,hash_join} options
and remove all product and test code associated with them.
Change-Id: I5ce2236d37c0ced188a4a81f7e00d4b8ac98e7e9
Reviewed-on: http://gerrit.cloudera.org:8080/7102
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
In order to conform to the ISO SQL specifications, this patch allows
the 'order by' clause to be optional for first_value() and last_value()
analytical functions.
Testing:
1. Added Analyzer tests
2. Added Planner tests which checks that a sort node is not used if both
'order by' and 'partition by' clauses are not used. Also, checks that
if only 'partition by' clause is used then the sorting is done only on
the partition column.
3. Added a query test that checks that the input is not reordered by
these analytic functions if 'order by' clause is not used
Change-Id: I5a3a56833ac062839629353ea240b361bc727d96
Reviewed-on: http://gerrit.cloudera.org:8080/7502
Reviewed-by: Matthew Jacobs <mj@cloudera.com>
Tested-by: Impala Public Jenkins
IMPALA-4000 added basic authorization support for Kudu
tables, but it had several limitations:
* Only the ALL privilege level can be granted to Kudu tables.
(Finer-grained levels such as only SELECT or only INSERT are
not supported.)
* Column level permissions on Kudu tables are not supported.
* Only users with ALL privileges on SERVER may create external
Kudu tables.
This patch relaxes the restrictions to allow:
* Allow column-level permissions
* Allow fine grained privileges SELECT and INSERT for those
statement types.
DELETE/UPDATE/UPSERT privileges now require ALL privileges
because Sentry will eventually get fine grained privilege
actions, and at that point Impala should support the more
specific actions (IMPALA-3840). The assumption is that the
Kudu table authorization support is currently so limited
that most users are not using this functionality yet, but
this is a behavior change that needs to be clearly stated in
the Impala release notes.
Testing: Adds FE and EE tests.
Change-Id: Ib12d2b32fa3e142e69bd8b0f24f53f9e5cbf7460
Reviewed-on: http://gerrit.cloudera.org:8080/7307
Reviewed-by: Matthew Jacobs <mj@cloudera.com>
Tested-by: Impala Public Jenkins
The fix for IMPALA-1654 has broken the compute incremental stats child
query generation logic for general partition expressions. This commit
fixes it and also adds new queries to fix the test gap. These tests
fail consistently without the patch.
Change-Id: I227fc06f580eb9174f60ad0f515a3641cec19268
Reviewed-on: http://gerrit.cloudera.org:8080/7379
Reviewed-by: Bharath Vissapragada <bharathv@cloudera.com>
Tested-by: Impala Public Jenkins
In a recent patch (IMPALA-5036) a bug was introduced where a count(*)
query with a group by a string partition column returned incorrect
results. Data was being written into the tuple at an incorrect offset.
Testing:
- Added an end to end test where we are selecting from a table
partitioned by string.
Change-Id: I225547574c2b2259ca81cb642d082e151f3bed6b
Reviewed-on: http://gerrit.cloudera.org:8080/7481
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
Impala currently supports total sorts (the entire set of data
is sorted) and top-n sorts (only the highest/lowest n elements
are sorted). This patch adds the ability to do partial sorts,
where the data is divided up into some number of subsets, each
of which is sorted individually.
It accomplishes this by adding a new exec node, PartialSortNode.
When PartialSortNode::GetNext() is called, it retrieves input
up to the query memory limit, uses the existing Sorter class to sort
it, and outputs it. This is faster than a total sort with SortNode
as it avoids the need to spill if the input is larger than the
memory limit.
Future work will look into setting a more restrictive memory limit
on the PartialSortNode. (IMPALA-5669)
In the planner, the SortNode plan node is used, with an enum value
indicating if it is a total or partial sort.
This also adds a new counter 'RunSize' to the runtime profile which
tracks the min, max, and avg size of the generated runs, in tuples.
As a first use case, partial sort is used where a total sort was
used previously for inserts/upserts into Kudu tables only. Future
work can extend this to other table sinks. (IMPALA-5649)
Testing:
- E2E test with a large INSERT into a Kudu table with a mem limit.
Checks that no spills occurred.
- Updated planner tests.
- Existing E2E tests and stress test verify correctness of INSERT.
- Perf tests on the 10 node cluster: inserting tpch_100.lineitem
into a Kudu table with mem_limit=3gb:
Previously: 5 runs are spilled, sort took 7m33s
Now: no spills, sort takes 6m19s, for ~18% speedup
Change-Id: Ieec2a15a0cc5240b1c13682067ab64670d1e0a38
Reviewed-on: http://gerrit.cloudera.org:8080/7267
Reviewed-by: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Tested-by: Impala Public Jenkins
Creating Kudu clients is very expensive as each will fetch
metadata from the Kudu master, so we should minimize the
number of Kudu clients that get created.
This patch stores a map from Kudu master addressed to Kudu
clients in KuduUtil to be used across the FE and catalog.
Another patch has already addressed the BE.
Future work will consider providing a way to invalidate
the stored Kudu clients in case something goes wrong
(IMPALA-5685)
This relies on two changes on the Kudu side: one that clears
non-covered range entries from the client's cache on table
open (d07ecd6ded01201c912d2e336611a6a941f48d98), and one
that automatically refreshes auth tokens when they expire
(603c1578c78c0377ffafdd9c427ebfd8a206bda3).
This patch disables some tests that no longer work as
they relied on Kudu metadata loading operations timing out,
but since we're reusing clients the metadata is already
loaded when the test is run.
Testing:
- Ran a stress test on a 10 node cluster: scan of a small
Kudu table, 1000 concurrent queries, load on the Kudu
master was reduced signficantly, from ~50% cpu to ~5%.
(with the BE changes included)
- Ran the Kudu e2e tests.
- Manually ran a test with concurrent INSERTs and
'ALTER TABLE ADD PARTITION' (which is affected by the
Kudu side change mentiond above) and verified
correctness.
Change-Id: I9b0b346f37ee43f7f0eefe34a093eddbbdcf2a5e
Reviewed-on: http://gerrit.cloudera.org:8080/6898
Reviewed-by: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Tested-by: Impala Public Jenkins
Privileges granted to a role assigned to a db/table whose name
contains upper case characters can disappear after a few seconds.
A privilege is inserted into the catalogObjectCache using a key
that uses the db/table name. The key gets converted to a lower
case before inserting.
Privilege name returned by sentryProxy is always lower case,
which might not match the privilegeName built in the catalog.
This triggers an update of the catalog object followed by a
removal of the old object. Since they both use the same key
in lower case it ends up deleting the newly updated object.
This change also adds a new catalogd startup option
(sentry_catalog_polling_frequency)
to configure the frequency at which catalogd polls the sentry service
to update any policy changes. The default value is 60 seconds.
Test:
Added a test which adds select privileges to 3 tables and dbs specified
in lower case, upper case and mixed case. The test verifies that the
privileges on the 3 tables do not disappear on a sentry update.
Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Reviewed-on: http://gerrit.cloudera.org:8080/7332
Reviewed-by: Matthew Jacobs <mj@cloudera.com>
Tested-by: Impala Public Jenkins
Bugs:
- FunctionCallExpr's toSql() doesn't include IGNORE NULLS if present
causing view definitions to break and leading to incorrect results.
- FunctionCallExpr's clone() implementation doesn't carry forward
IGNORE NULLS option if present. One case that breaks with this is
querying views containing analytic exprs causing wrong plans.
Fixed both the bugs and added a test that can reliably reproduce this.
Change-Id: I723897886c95763c3f29d6f24c4d9e7d43898ade
Reviewed-on: http://gerrit.cloudera.org:8080/7416
Reviewed-by: Bharath Vissapragada <bharathv@cloudera.com>
Tested-by: Impala Public Jenkins
Kudu tables did not treat some table properties correctly.
Change-Id: I69fa661419897f2aab4632015a147b784a6e7009
Reviewed-on: http://gerrit.cloudera.org:8080/7454
Reviewed-by: Matthew Jacobs <mj@cloudera.com>
Tested-by: Impala Public Jenkins
The -use_local_tz_for_unix_timestamp_conversion flag exists
to specify if TIMESTAMPs should be interpreted as localtime
or UTC when converting to/from Unix time via builtins:
from_unixtime(bigint unixtime)
unix_timestamp(string datetime[, ...])
unix_timestamp(timestamp datetime)
However, the KuduScanner was calling into code that, when
the gflag above was set, interpreted Unix times as local
time. Unfortunately the write path (KuduTableSink) and some
FE TIMESTAMP code (see KuduUtil.java) did not have this
behavior, i.e. we were handling the gflag inconsistently.
Tests:
* Adds a custom cluster test to run Kudu test cases with
-use_local_tz_for_unix_timestamp_conversion.
* Adds tests for the new builtin
unix_micros_to_utc_timestamp() which run in a custom
cluster test (added test_local_tz_conversion.py) as well
as in the regular tests (added to test_exprs.py).
Change-Id: I423a810427353be76aa64442044133a9a22cdc9b
Reviewed-on: http://gerrit.cloudera.org:8080/7311
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
The following use of sequence file writer can lead to a crash:
> set compression_codec=gzip;
> set seq_compression_mode=record;
> set allow_unsupported_formats=1;
> create table seq_tbl like tbl stored as sequencefile;
> insert into seq_tbl select * from tbl;
This fix removes the MemPool::FreeAll() call from
HdfsSequenceTableWriter::Flush(). Freeing the memory pool in Flush()
is incorrect because a memory pool buffer is cached by the compressor
in the table writer which isn't reset across calls to Flush().
If the file that is being written is big enough,
HdfsSequenceTableWriter::AppendRows() will call Flush() multiple
times causing memory corruption.
Change-Id: Ida0b9f189175358ae54149d0e1af7caa06ae3bec
Reviewed-on: http://gerrit.cloudera.org:8080/7394
Reviewed-by: Michael Ho <kwho@cloudera.com>
Tested-by: Impala Public Jenkins
This moves away from the PipelinedPlanNodeSet approach of enumerating
sets of concurrently-executing nodes because unions would force
creating many overlapping sets of nodes. The new approach computes
the peak resources during Open() and the peak resources between Open()
and Close() (i.e. while calling GetNext()) bottom-up for each plan node
in a fragment. The fragment resources are then combined to produce the
query resources.
The basic assumptions for the new resource estimates are:
* resources are acquired during or after the first call to Open()
and released in Close().
* Blocking nodes call Open() on their child before acquiring
their own resources (this required some backend changes).
* Blocking nodes call Close() on their children before returning
from Open().
* The peak resource consumption of the query is the sum of the
independent fragments (except for the parallel join build plans
where we can assume there will be synchronisation). This is
conservative but we don't synchronise fragment Open() and Close()
across exchanges so can't make stronger assumptions in general.
Also compute the sum of minimum reservations. This will be useful
in the backend to determine exactly when all of the initial
reservations have been claimed from a shared pool of initial reservations.
Testing:
* Updated planner tests to reflect behavioural changes.
* Added extra resource requirement planner tests for unions, subplans,
pipelines of blocking operators, and bushy join plans.
* Added single-node plans to resource-requirements tests. These have
more complex plan trees inside a single fragment, which is useful
for testing the peak resource requirement logic.
Change-Id: I492cf5052bb27e4e335395e2a8f8a3b07248ec9d
Reviewed-on: http://gerrit.cloudera.org:8080/7223
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
IMPALA-4120 fixed an issue where lead/lag was potentially
operating on memory that the UDA didn't own, resulting in
potentially wrong results. As part of that fix, lead and lag
started allocating 'global' UDF memory (e.g. via Allocate()
rather than AllocateLocal()) in Init() which needs to be freed
in Serialize() or Finalize(), but only lead() was updated to
free the memory. This memory is eventually freed when the
fragment is torn down, but as a result of not freeing the
memory in Serialize or Finalize, the memory may be allocated
longer than necessary.
Change-Id: Id2b69b4ccb9cac076abca19bed6f0b1dd11dfff3
Reviewed-on: http://gerrit.cloudera.org:8080/7371
Reviewed-by: Matthew Jacobs <mj@cloudera.com>
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Tested-by: Impala Public Jenkins
In commit 741421de, we accidently made it so that is_null=true
StringVals became is_null=false with len=0. Fix that and add
a regression test.
Change-Id: I34d288aad66a2609484058c9a177c02200cb6a6e
Reviewed-on: http://gerrit.cloudera.org:8080/7364
Reviewed-by: Bharath Vissapragada <bharathv@cloudera.com>
Tested-by: Impala Public Jenkins
IMPALA-3742 introduced KuduPartitionExpr, which takes a row and passes
it to the Kudu client to determine what partitionit belongs to.
The DataStreamSender never frees the local allocations for the Kudu
partition exprs causing it to hang on to memory longer than it needs to.
This patch also fixes two other related issues:
- DataStreamSender was dropping the Status from AddRow in the Kudu
branch. Adds 'RETURN_IF_ERROR' and 'WARN_UNUSED_RESULT'
- Changes the HASH case in DataStreamSender to call FreeLocalAllocations
on a per-batch basis, instead of a per-row basis.
Testing:
- Added an e2e test that runs a large insert with a mem limit that
failed with oom previously.
Change-Id: Ia661eb8bed114070728a1497ccf7ed6893237e5e
Reviewed-on: http://gerrit.cloudera.org:8080/7346
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Reviewed-by: Michael Ho <kwho@cloudera.com>
Tested-by: Impala Public Jenkins
Instead of materializing empty rows when computing count star, we use
the data stored in the Parquet RowGroup.num_rows field. The Parquet
scanner tuple is modified to have one slot into which we will write the
num rows statistic. The aggregate function is changed from count to a
special sum function that gets initialized to 0. We also add a rewrite
rule so that count(<literal>) is rewritten to count(*) in order to make
sure that this optimization is applied in all cases.
Testing:
- Added functional and planner tests
Change-Id: I536b85c014821296aed68a0c68faadae96005e62
Reviewed-on: http://gerrit.cloudera.org:8080/6812
Reviewed-by: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Tested-by: Impala Public Jenkins
Reworks the FK/PK join detection logic to:
- more accurately recognize many-to-many joins
- avoid dim/dim joins for multi-column PKs
The new detection logic maintains our existing philosophy of generally
assuming a FK/PK join, unless there is strong evidence to the
contrary, as follows.
For each set of simple equi-join conjuncts between two tables, we
compute the joint NDV of the right-hand side columns by
multiplication, and if the joint NDV is significantly smaller than
the right-hand side row count, then we are fairly confident that the
right-hand side is not a PK. Otherwise, we assume the set of conjuncts
could represent a FK/PK relationship.
Extends the explain plan to include the outcome of the FK/PK detection
at EXPLAIN_LEVEL > STANDARD.
Performance testing:
1. Full TPC-DS run on 10TB:
- Q10 improved by >100x
- Q72 improved by >25x
- Q17,Q26,Q29 improved by 2x
- Q64 regressed by 10x
- Total runtime: Improved by 2x
- Geomean: Minor improvement
The regression of Q64 is understood and we will try to address it
in follow-on changes. The previous plan was better by accident and
not because of superior logic.
2. Nightly TPC-H and TPC-DS runs:
- No perf differences
Testing:
- The existing planner test cover the changes.
- Code/hdfs run passed.
Change-Id: I49074fe743a28573cff541ef7dbd0edd88892067
Reviewed-on: http://gerrit.cloudera.org:8080/7257
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Impala Public Jenkins
This is done to simplify the CHAR(N) logic. I believe this is overall an
improvement - any benefits of the out-of-line storage that motivated
this optimisation originally were outweighed by the added complexity.
This also avoids IMPALA-5559 (fe/be have different notions of var-len),
which will unblock IMPALA-3200.
Pros:
* Reduce the number of code paths and improve test coverage.
(e.g. avoids IMPALA-5559: fe/be have different notions of var-len)
* Reduced memory to store non-NULL data (saves 12-byte StringValue)
* Fewer branches in code -> save CPU cycles.
* If CHAR(N) performance is important, reduced complexity makes it
easier to implement codegen.
Cons:
* Requires N bytes to store a NULL value.
* May hurt cache locality (although this is speculative in my mind).
The change is mostly mechanical - I removed MAX_CHAR_INLINE_LENGTH
and then removed branches that depended on that.
Testing:
Ran exhaustive build.
Change-Id: I9c0b823ccff6b0c37f5267c548d096c29b8caac3
Reviewed-on: http://gerrit.cloudera.org:8080/7303
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
This is similar to the single-node execution optimisation, but applies
to slightly larger queries that should run in a distributed manner but
won't benefit from codegen.
This adds a new query option disable_codegen_rows_threshold that
defaults to 50,000. If fewer than this number of rows are processed
by a plan node per impalad, the cost of codegen almost certainly
outweighs the benefit.
Using rows processed as a threshold is justified by a simple
model that assumes the cost of codegen and execution per row for
the same operation are proportional. E.g. if x is the complexity
of the operation, n is the number of rows processed, C is a
constant factor giving the cost of codegen and Ec/Ei are constant
factor giving the cost of codegen'd and interpreted execution and
d, then the cost of the codegen'd operator is C * x + Ec * x * n
and the cost of the interpreted operator is Ei * x * n. Rearranging
means that interpretation is cheaper if n < C / (Ei - Ec), i.e. that
(at least with the simplified model) it makes sense to choose
interpretation or codegen based on a constant threshold. The
model also implies that it is somewhat safer to choose codegen
because the additional cost of codegen is O(1) but the additional
cost of interpretation is O(n).
I ran some experiments with TPC-H Q1, varying the input table size, to
determine what the cut-over point where codegen was beneficial was.
The cutover was around 150k rows per node for both text and parquet.
At 50k rows per node disabling codegen was very beneficial - around
0.12s versus 0.24s. To be somewhat conservative I set the default
threshold to 50k rows. On more complex queries, e.g. TPC-H Q10, the
cutover tends to be higher because there are plan nodes that process
many fewer than the max rows.
Fix a couple of minor issues in the frontend - the numNodes_
calculation could return 0 for Kudu, and the single node optimization
didn't handle the case where for a scan node with conjuncts, a limit
and missing stats correctly (it considered the estimate still valid.)
Testing:
Updated e2e tests that set disable_codegen to set
disable_codegen_rows_threshold to 0, so that those tests run both
with and without codegen still.
Added an e2e test to make sure that the optimisation is applied in
the backend.
Added planner tests for various cases where codegen should and shouldn't
be disabled.
Perf:
Added a targeted perf test for a join+agg over a small input, which
benefits from this change.
Change-Id: I273bcee58641f5b97de52c0b2caab043c914b32e
Reviewed-on: http://gerrit.cloudera.org:8080/7153
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
The parser didn't account for the possibility of negative
numeric literals.
Testing:
Added a test that sets a negative value. Query tests send the whole
"set" statement to the backend for execution so exercise the parser.
Ran core tests.
Change-Id: I5c415dbed6ba1122919be75f5811444d88ee03b4
Reviewed-on: http://gerrit.cloudera.org:8080/7316
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
The bug was in the DCHECK. The DCHECK is intended to make sure that a
tuple's string data didn't get split across blocks. The logic assumed
that if the second-or-later string column was in the next-block, that
the strings were split between blocks. However, that assumption is
invalid if there are NULL strings, which do not belong in any block.
The fix for the DCHECK (which is still useful) is to count the number
of non-NULL strings and make sure that no non-NULL strings were split
between blocks.
Testing:
Added a test that reproduces the crash.
Change-Id: I7a8dee982501008efff5b5abc192cfb5e6544a90
Reviewed-on: http://gerrit.cloudera.org:8080/7295
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
Hash join node currently does not apply the limits correctly.
This issue gets masked most of the times since the planner sticks
an exchange node on top of most of the joins. This issue gets
exposed when NUM_NODES=1.
Change-Id: I414124f8bb6f8b2af2df468e1c23418d05a0e29f
Reviewed-on: http://gerrit.cloudera.org:8080/6778
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
Kudu recently added the ability to alter a column's default value
and storage attributes (KUDU-861). This patch adds the ability to
modify these from Impala using ALTER.
It also supports altering a column's comment for non-Kudu tables.
It does not support setting a column to be a primary key or
changing a column's nullability, because those are not supported on
the Kudu side yet.
Syntax:
ALTER TABLE <table> ALTER [COLUMN] <column>
SET <attr> <value> [<attr> <value> [<attr> <value>...]]
where <attr> is one of:
- DEFAULT, BLOCK_SIZE, ENCODING, COMPRESSION (Kudu tables)
- COMMENT (non-Kudu tables)
ALTER TABLE <table> ALTER [COLUMN] <column> DROP DEFAULT
This is similar to the existing CHANGE statement:
ALTER TABLE <table> CHANGE <column> <new_col_name> <type>
[COMMENT <comment>]
but the new syntax is more natural for setting column properties
when the column name and type are not being changed. Both ALTER
COLUMN and CHANGE COLUMN operations use AlterTableAlterColStmt and
are sent to the catalog as ALTER_COLUMN operations.
Testing:
- Added FE tests to ParserTest and AnalyzeDDLTest
- Added EE tests to test_kudu.py
Change-Id: Id2e8bd65342b79644a0fdcd925e6f17797e89ad6
Reviewed-on: http://gerrit.cloudera.org:8080/6955
Reviewed-by: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Tested-by: Impala Public Jenkins
The bug is that the join tried to bring the next spilled partition into
memory while still holding onto memory from the current partition.
The fix is to return earlier if the output batch is at capacity so
that resources are flushed.
Also reduce some of the redundancy in the loop that drives the spilling
logic and catch some dropped statuses..
Testing:
The failure was originally reproduced by my IMPALA-4703 patch. I was
able to cause a query failure with the current code by reducing the
memory limit for an existing query. Before it failed with up to 12MB of
memory. Now it succeeds with 8MB or less.
Ran exhaustive build.
Change-Id: I075388d348499c5692d044ac1bc38dd8dd0b10c7
Reviewed-on: http://gerrit.cloudera.org:8080/7180
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Tested-by: Impala Public Jenkins
This patch aligns the sorter's methods closer with the ExecNode methods
and moves the possibly-failing parts of Reset() into Open().
Testing:
Added WARN_UNUSED_RESULT to all the sorter methods that return Status to
prevent similar issues in future.
Add a test that sometimes goes down this code path. It was able to cause
a crash at least once every 5 executions.
Ran an exhaustive build to make sure there were no other regressions.
Change-Id: I7d4f9e93a44531901e663b3f1e18edc514363f74
Reviewed-on: http://gerrit.cloudera.org:8080/7134
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
A recent addition to test_create_table_like_file (IMPALA-2525)
relies on a file, enum.parquet, being preloaded into HDFS, which
is done by create-load-data.sh.
The problem is that the test creates the table as an internal
table with its location as the directory containing enum.parquet.
When the test completes and the table is dropped, enum.parquet
is deleted, so the test cannot be successfully run again, and a
snapshot generated from the contents of HDFS afterwards will
not contain the file.
The fix is to create the table as an external table.
Testing:
- Ran the test and verfied enum.parquet is still present in HDFS.
Change-Id: I6c386843e5ef5bf6fc208db1ff90be98fd8baacf
Reviewed-on: http://gerrit.cloudera.org:8080/7139
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Impala Public Jenkins
Codegen time under ASAN can take ~10s, making the 15s timeouts for
runtime filter tests a bit small. Double those timeouts to 30s.
Change-Id: I2280e08910430e271da2173e465731bba5aef6cf
Reviewed-on: http://gerrit.cloudera.org:8080/7097
Reviewed-by: Michael Ho <kwho@cloudera.com>
Tested-by: Impala Public Jenkins
This change executes the tests added to subplans.test and removes
a test which incorrectly references subplannull_data.test (a file
which does not exist)
Change-Id: I02b4f47553fb8f5fe3425cde2e0bcb3245c39b91
Reviewed-on: http://gerrit.cloudera.org:8080/7038
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Impala Public Jenkins
The bug was that the constant exprs of a union were only
evaluated for the first fragment instance. However, for
a union inside a subplan, we should always evaluate the
constant exprs.
Testing:
- Added a regression test.
- Locally ran test_nested_types.py and the union tests in
test_queries.py
Change-Id: Icd2f21f0213188e2304f8e9536019c7940c07768
Reviewed-on: http://gerrit.cloudera.org:8080/7091
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Impala Public Jenkins
For every new iteration of a subplan there are leftover
rows from the previous iteration of a subplan. This change
transfers the ownership from the probe_batch_ to output_batch_
and resets the probe_batch_ on hitting the limit.
Change-Id: Iafd621d33a4e2fac42391504566ffd8dd0e18a67
Reviewed-on: http://gerrit.cloudera.org:8080/7014
Tested-by: Impala Public Jenkins
Reviewed-by: Lars Volker <lv@cloudera.com>
While support for TIMESTAMP columns in Kudu tables has been
committed (IMPALA-5137), it does not support TIMESTAMP
column default values.
This supports CREATE TABLE syntax to specify the default
values, but more importantly this fixes the loading of Kudu
tables that may have had default values set on
UNIXTIME_MICROS columns, e.g. if the table was created via
the python client. This involves fixing KuduColumn to hide
the LiteralExpr representing the default value because it
will be a BIGINT if the column type is TIMESTAMP. It is only
needed to call toSql() and toStringValue(), so helper
functions are added to KuduColumn to encapsulate special
logic for TIMESTAMP.
TODO: Add support and tests for ALTER setting the default
value (when IMPALA-4622 is committed).
Change-Id: I655910fb4805bb204a999627fa9f68e43ea8aaf2
Reviewed-on: http://gerrit.cloudera.org:8080/6936
Reviewed-by: Matthew Jacobs <mj@cloudera.com>
Tested-by: Impala Public Jenkins
This change adds a query option to disable reading Parquet statistics.
It provides a workaround when dealing with files that have corrupt
parquet statistics.
Note that Impala handles Parquet files affected by PARQUET-251 correctly
by ignoring statistics for anything but plain numeric types. This query
option is supposed to help with files affected by unknown or errors or
by errors that are yet to be made.
Change-Id: I427f7fde40d0f4b703751e40f3c2109a850643f7
Reviewed-on: http://gerrit.cloudera.org:8080/7001
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Tested-by: Impala Public Jenkins
The current code only tests with the default setting
for parquet_dictionary_filtering, which is true. This
adds a test to verify that parquet_dictionary_filtering
set to false does not filter any row groups.
Change-Id: If3175ce1d01c806d822c2782d60ca10939e7179e
Reviewed-on: http://gerrit.cloudera.org:8080/7021
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Impala Public Jenkins
This change loads the missing tables in TPC-DS. In addition,
it also fixes up the loading of the partitioned table store_sales
so all partitions will be loaded. The existing TPC-DS queries are
also updated to use the parameters for qualification runs as noted
in the TPC-DS specification. Some hard-coded partition filters were
also removed. They were there due to the lack of dynamic partitioning
in the past. Some missing TPC-DS queries are also added to this change,
including query28 which discovered the infamous IMPALA-5251.
Having all tables in TPC-DS available paves the way for us to include
all supported TPCDS queries in our functional testing. Due to the change
in the data, planner tests and the E2E tests have different results than
before. The results of E2E tests were compared against the run done with
Netezza and Vertica. The divergence were all due to the truncation behavior
of decimal types in DECIMAL_V1.
Change-Id: Ic5277245fd20827c9c09ce5c1a7a37266ca476b9
Reviewed-on: http://gerrit.cloudera.org:8080/6877
Reviewed-by: Michael Brown <mikeb@cloudera.com>
Tested-by: Impala Public Jenkins
The main idea of this patch is to use table stats to
extrapolate the row counts for new/modified partitions.
Existing behavior:
- Partitions that lack the row count stat are ignored
when estimating the cardinality of HDFS scans. Such
partitions effectively have an estimated row count
of zero.
- We always use the row count stats for partitions that
have one. The row count may be innaccurate if data in
such partitions has changed significantly.
Summary of changes:
- Enhance COMPUTE STATS to also store the total number
of file bytes in the table.
- Use the table-level row count and file bytes stats
to estimate the number of rows in a scan.
- A new impalad startup flag is added to enable/disable
the extrapolation behavior. The feature is disabled by
default. Note that even with the feature disabled,
COMPUTE STATS stores the file bytes so you can enable
the feature without having to run COMPUTE STATS again.
Testing:
- Added new FE unit test
- Added new EE test
Change-Id: I972c8a03ed70211734631a7dc9085cb33622ebc4
Reviewed-on: http://gerrit.cloudera.org:8080/6840
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Impala Public Jenkins
Most of the code changes are to restructure things so that the
scratch batch's tuple buffer is stored in a separate MemPool
from auxiliary memory such as decompression buffers. This part
of the change does not change the behaviour of the scanner in
itself, but allows us to recycle the tuple buffer without holding
onto unused auxiliary memory.
The optimisation is implemented in TryCompact(): if enough rows
were filtered out during the copy from the scratch batch to the
output batch, the fixed-length portions of the surviving rows
(if any) are copied to a new, smaller, buffer, and the original,
larger, buffer is reused for the next scratch batch.
Previously the large buffer was always attached to the output batch,
so a large buffer was transferred between threads for every scratch
batch processed. In combination with the decompression buffer change
in IMPALA-5304, this means that in many cases selective scans don't
produce nearly as many empty or near-empty batches and do not attach
nearly as much memory to each batch.
Performance:
Even on an 8 core machine I see some speedup on selective scans.
Profiling with "perf top" also showed that time in TCMalloc
was reduced - it went from several % of CPU time to a minimal
amount.
Running TPC-H on the same machine showed a ~5% overall improvement
and no regressions. E.g. Q6 got 20-25% faster.
I hope to do some additional cluster benchmarking on systems
with more cores to verify that the severe performance problems
there are fixed, but in the meantime it seems like we have enough
evidence that it will at least improve things.
Testing:
Add a couple of selective scans that exercise the new code paths.
Change-Id: I3773dc63c498e295a2c1386a15c5e69205e747ea
Reviewed-on: http://gerrit.cloudera.org:8080/6949
Reviewed-by: Tim Armstrong <tarmstrong@cloudera.com>
Tested-by: Impala Public Jenkins
The sortby() hint is superseded by the SORT BY SQL clause, which has
been introduced in IMPALA-4166. This changes removes the hint.
Change-Id: I83e1cd6fa7039035973676322deefbce00d3f594
Reviewed-on: http://gerrit.cloudera.org:8080/6885
Reviewed-by: Lars Volker <lv@cloudera.com>
Tested-by: Impala Public Jenkins
Adds support in DDL for timestamps in Kudu range partition syntax.
For convenience, strings can be specified with or without
explicit casts to TIMESTAMP.
E.g.
create table ts_ranges (ts timestamp primary key, i int)
partition by range (
partition '2009-01-02 00:00:00' <= VALUES < '2009-01-03 00:00:00'
) stored as kudu
Range bounds are converted to Kudu UNIXTIME_MICROS during
analysis.
Testing: Adds FE and EE tests.
Change-Id: Iae409b6106c073b038940f0413ed9d5859daaeff
Reviewed-on: http://gerrit.cloudera.org:8080/6849
Reviewed-by: Matthew Jacobs <mj@cloudera.com>
Tested-by: Impala Public Jenkins
This change builds on the support for reading and writing
TIMESTAMP columns to Kudu tables (see [1]), adding support
for pushing TIMESTAMP predicates to Kudu for scans.
Binary predicates and IN list predicates are supported.
Testing: Added some planner and EE tests to validate the
behavior.
1: https://gerrit.cloudera.org/#/c/6526/
Change-Id: I08b6c8354a408e7beb94c1a135c23722977246ea
Reviewed-on: http://gerrit.cloudera.org:8080/6789
Reviewed-by: Matthew Jacobs <mj@cloudera.com>
Tested-by: Impala Public Jenkins
This change adds support for adding SORT BY (...) clauses to CREATE
TABLE and ALTER TABLE statements. Examples are:
CREATE TABLE t (i INT, j INT, k INT) PARTITIONED BY (l INT) SORT BY (i, j);
CREATE TABLE t SORT BY (int_col,id) LIKE u;
CREATE TABLE t LIKE PARQUET '/foo' SORT BY (id,zip);
ALTER TABLE t SORT BY (int_col,id);
ALTER TABLE t SORT BY ();
Sort columns can only be specified for Hdfs tables and effectiveness may
vary based on storage type; for example TEXT tables will not see
improved compression. The SORT BY clause must not contain clustering
columns. The columns in the SORT BY clause are stored in the
'sort.columns' table property and will result in an additional SORT node
being added to the plan before the final table sink. Specifying sort
columns also enables clustering during inserts, so the SORT node will
contain all partitioning columns first, followed by the sort columns. We
do this because sort columns add a SORT node to the plan and adding the
clustering columns to the SORT node is cheap.
Sort columns supersede the sortby() hint, which we will remove in a
subsequent change (IMPALA-5144). Until then, it is possible to specify
sort columns using both ways at the same time and the column lists
will be concatenated.
Change-Id: I08834f38a941786ab45a4381c2732d929a934f75
Reviewed-on: http://gerrit.cloudera.org:8080/6495
Reviewed-by: Lars Volker <lv@cloudera.com>
Tested-by: Impala Public Jenkins
Adds Impala support for TIMESTAMP types stored in Kudu.
Impala stores TIMESTAMP values in 96-bits and has nanosecond
precision. Kudu's timestamp is a 64-bit microsecond delta
from the Unix epoch (called UNIXTIME_MICROS), so a conversion
is necessary.
When writing to Kudu, TIMESTAMP values in nanoseconds are
averaged to the nearest microsecond.
When reading from Kudu, the KuduScanner returns
UNIXTIME_MICROS with 8bytes of padding so Impala can convert
the value to a TimestampValue in-line and copy the entire
row.
Testing:
Updated the functional_kudu schema to use TIMESTAMPs instead
of converting to STRING, so this provides some decent
coverage. Some BE tests were added, and some EE tests as
well.
TODO: Support pushing down TIMESTAMP predicates
TODO: Support TIMESTAMPs in range partitioning expressions
Change-Id: Iae6ccfffb79118a9036fb2227dba3a55356c896d
Reviewed-on: http://gerrit.cloudera.org:8080/6526
Reviewed-by: Matthew Jacobs <mj@cloudera.com>
Tested-by: Impala Public Jenkins
An INSERT into a Kudu table with a constant value being inserted
into a partition column causes an IllegalStateExcaption. This is
because DistributedPlanner removes constants from the list of
partition exprs before creating the KuduPartitionExpr, but
KuduPartitionExpr expects to get one expr per partition column.
The fix is to pass the full list of partition exprs into the
KuduPartitionExpr, instead of the list that has had constants
removed. This preserves the behavior that if all of the partition
exprs are constant we fall back to UNPARTITIONED.
One complication is that if a partition expr is a NullLiteral, it
must be cast to a specific type to be passed to the BE. The
InsertStmt will cast the partition exprs to the partition column
types, but these casts may be lost from the copies of the partition
exprs stored by the KuduPartitionExpr during reset(). To fix this,
the KuduPartitionExpr can store the types of the partition cols and
recast the partition exprs to those types during analyze().
Change-Id: I12cbb319f9a5c47fdbfee347b47650186b27f8f9
Reviewed-on: http://gerrit.cloudera.org:8080/6828
Reviewed-by: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Tested-by: Impala Public Jenkins
This change adds functionality to write and read parquet::Statistics for
Decimal, String, and Timestamp values. As an exception, we don't read
statistics for CHAR columns, since CHAR support is broken in Impala
(IMPALA-1652).
This change also switches from using the deprecated fields 'min' and
'max' to populate the new fields 'min_value' and 'max_value' in
parquet::Statistics, that were added in parquet-format pull request #46.
The HdfsParquetScanner will preferably read the new fields if they are
populated and if the column order 'TypeDefinedOrder' has been used to
compute the statistics. For columns without a column order set or with
only the deprecated fields populated, the scanner will read them only if
they are of simple numeric type, i.e. boolean, integer, or floating
point.
This change removes the validation of the Parquet Statistics we write to
Hive from the tests, since Hive does not write the new fields. Instead
it adds a parquet file written by Hive that uses the deprecated fields
for its statistics. It uses that file to exercise the fallback logic for
supported types in a test.
This change also cleans up the interface of ParquetPlainEncoder in
parquet-common.h.
Change-Id: I3ef4a5d25a57c82577fd498d6d1c4297ecf39312
Reviewed-on: http://gerrit.cloudera.org:8080/6563
Reviewed-by: Lars Volker <lv@cloudera.com>
Tested-by: Lars Volker <lv@cloudera.com>
This change fixed IMPALA-4873 by adding the capability to supply a dict
'test_file_vars' to run_test_case(). Keys in this dict will be replaced
with their values inside test queries before they are executed.
Change-Id: Ie3f3c29a42501cfb2751f7ad0af166eb88f63b70
Reviewed-on: http://gerrit.cloudera.org:8080/6817
Reviewed-by: Michael Brown <mikeb@cloudera.com>
Tested-by: Impala Public Jenkins
This generates min/max predicates for InPredicates that
have only constant values in the IN list. It is only
used for statistics filtering on Parquet files.
Change-Id: I4a88963a7206f40a867e49eceeaf03fdd4f71997
Reviewed-on: http://gerrit.cloudera.org:8080/6810
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Impala Public Jenkins
The slot descriptor vectors are not guaranteed to be sorted on the slot
index within a tuple. As a result, TupleDescriptor::LayoutEquals()
sometimes returned a wrong result.
In this patch, we sort the vectors of slot descriptors on the slot index
within the tuple before comparing the vectors.
Testing:
- ran EE tests locally.
Change-Id: I426ad244678dbfe517262dfb7bbf4adc0247a35e
Reviewed-on: http://gerrit.cloudera.org:8080/6610
Reviewed-by: Dan Hecht <dhecht@cloudera.com>
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Impala Public Jenkins
Bulk DMLs (INSERT, UPSERT, UPDATE, and DELETE) for Kudu
are currently painful because we just send rows randomly,
which creates a lot of work for Kudu since it partitions
and sorts data before writing, causing writes to be slow
and leading to timeouts.
We can alleviate this by sending the rows to Kudu already
partitioned and sorted. This patch partitions and sorts
rows according to Kudu's partitioning scheme for INSERTs
and UPSERTs. A followup patch will handle UPDATE and DELETE.
It accomplishes this by inserting an exchange node and a sort
node into the plan before the operation. Both the exchange and
the sort are given a KuduPartitionExpr which takes a row and
calls into the Kudu client to return its partition number.
It also disallows INSERT hints for Kudu tables, since the
hints that we support (SHUFFLE, CLUSTER, SORTBY), so longer
make sense.
Testing:
- Updated planner tests.
- Ran the Kudu functional tests.
- Ran performance tests demonstrating that we can now handle much
larger inserts without having timeouts.
Change-Id: I84ce0032a1b10958fdf31faef225372c5c38fdc4
Reviewed-on: http://gerrit.cloudera.org:8080/6559
Reviewed-by: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Tested-by: Impala Public Jenkins
