Skip to content

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Feb 16, 2022

Which issue does this PR close?

N/A

Rationale for this change

In an attempt to speed up development iteration, I am running this command

cargo test --no-default-features -p datafusion

I see several warnings

warning: unused variable: `expr`
    --> datafusion/src/sql/planner.rs:1585:17
     |
1585 |                 expr,
     |                 ^^^^ help: try ignoring the field: `expr: _`
     |
     = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `substring_from`
    --> datafusion/src/sql/planner.rs:1586:17
     |
1586 |                 substring_from,
     |                 ^^^^^^^^^^^^^^ help: try ignoring the field: `substring_from: _`

warning: unused variable: `substring_for`
    --> datafusion/src/sql/planner.rs:1587:17
     |
1587 |                 substring_for,
     |                 ^^^^^^^^^^^^^ help: try ignoring the field: `substring_for: _`

warning: `datafusion` (lib) generated 3 warnings

and several tests do not pass:

---- test_fn_translate stdout ----
Error: ArrowError(ExternalError(Internal("function translate requires compilation with feature flag: unicode_expressions.")))
thread 'test_fn_translate' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/test/src/lib.rs:195:5
stack backtrace:
   0: rust_begin_unwind
             at /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/std/src/panicking.rs:498:5
   1: core::panicking::panic_fmt
             at /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/core/src/panicking.rs:107:14
   2: core::panicking::assert_failed_inner
             at /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/core/src/panicking.rs:182:23
   3: core::panicking::assert_failed
             at /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/core/src/panicking.rs:145:5
   4: test::assert_test_result
             at /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/test/src/lib.rs:195:5
   5: dataframe_functions::test_fn_translate::{{closure}}
             at ./tests/dataframe_functions.rs:652:7
   6: core::ops::function::FnOnce::call_once
             at /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/core/src/ops/function.rs:227:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

What changes are included in this PR?

  1. Fix warnings
  2. cfg out tests that require unicode expressions

Are there any user-facing changes?

no

@alamb alamb added the development-process Related to development process of DataFusion label Feb 16, 2022
@github-actions github-actions bot added datafusion sql SQL Planner labels Feb 16, 2022
Copy link
Contributor Author

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find understanding the diff with https://github.com/apache/arrow-datafusion/pull/1845/files?w=1 is much easier

@alamb alamb marked this pull request as ready for review February 16, 2022 20:01
Ok(())
}

async fn generic_query_length<T: 'static + Array + From<Vec<&'static str>>>(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function is only used in this module

@alamb alamb merged commit 67fe623 into apache:master Feb 17, 2022
@alamb alamb deleted the alamb/clean_up_tests_without_default_features branch August 8, 2023 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

development-process Related to development process of DataFusion sql SQL Planner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants