Skip to content

Conversation

@jimexist
Copy link
Member

@jimexist jimexist commented Nov 1, 2021

Which issue does this PR close?

Closes #.

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

@github-actions github-actions bot added datafusion sql SQL Planner labels Nov 1, 2021
houqp
houqp previously approved these changes Nov 2, 2021
@Dandandan
Copy link
Contributor

I am not sure anymore this is a good idea.

The problem with introducing null as scalar is that we lose the type information on the scalar (making everything type Null). In order to make this work, the value should be converted to an array of the original type afterwards.

@houqp
Copy link
Member

houqp commented Nov 2, 2021

That's a good point @Dandandan

@houqp houqp dismissed their stale review November 2, 2021 06:39

losing type info

@jimexist
Copy link
Member Author

jimexist commented Nov 2, 2021

I am not sure anymore this is a good idea.

The problem with introducing null as scalar is that we lose the type information on the scalar (making everything type Null). In order to make this work, the value should be converted to an array of the original type afterwards.

The alternative I think is to infer types for null during planning

@alamb
Copy link
Contributor

alamb commented Nov 2, 2021

I think that during planning we should treat the type of any ScalarValue::*(None) as DataType::Null and coerce them appropriately to the needed types as part of physical planning.

@jimexist I think added the needed casting logic in arrow-rs at apache/arrow-rs#884 (though that didn't make it into 6.1.0 -- so I suppose we have to wait a bit longer -- though maybe I'll end up making a 6.2.0 before 6.1.0 gets through the voting process 🤣 )

Once we can cast from DataType::Null I think we'll be able to make this work in DataFusion

@liukun4515
Copy link
Contributor

any progress, is there any context or doc for this?
@jimexist

@alamb
Copy link
Contributor

alamb commented Dec 8, 2021

I had effectively changed Expr::Scalar to report its type as DataType::Null its interior ScalarValue::is_null() returned true. That seemed fairly promising but I got blocked by the lack of null cast kernels in Arrow.

Now I think we have those kernels it would be useful to try again -- but I am not sure when I will have the time again

@jimexist jimexist marked this pull request as draft December 28, 2021 15:29
@alamb
Copy link
Contributor

alamb commented Dec 29, 2021

I am trying to clean up the list of PRs to review in DataFusion so marking old ones as stale. Please let us know if you plan to work on this soon. Otherwise we will close it down and reopen it when the time is right.

@alamb alamb added the stale-pr label Dec 29, 2021
@alamb
Copy link
Contributor

alamb commented Jan 18, 2022

Closing stale PRs, please reopen if this was a mistake and you plan to keep working on this one

@alamb alamb closed this Jan 18, 2022
@jimexist jimexist deleted the add-untyped-null branch March 16, 2022 06:29
H0TB0X420 added a commit to H0TB0X420/datafusion that referenced this pull request Oct 7, 2025
…1242)

* Fix drop() method to handle quoted column names consistently

- Strip quotes from column names in drop() method
- Maintains consistency with other DataFrame operations
- Both drop('col') and drop('col') now work

Fixes apache#1212

* Update drop() method docstring to clarify quote handling

- Document that column names are case-sensitive and don't require quotes
- Clarify that both quoted and unquoted column names are accepted
- Add examples showing both 'col' and 'col' syntax work
- Note difference from select() operation behavior

* Fix whitespace and documentation errors

---------

Co-authored-by: Tim Saucer <timsaucer@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sql SQL Planner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants