Skip to content

[SPARK-21957][SQL] Add current_user function - #19169

Closed
mgaido91 wants to merge 1 commit into
apache:masterfrom
mgaido91:SPARK-21957
Closed

[SPARK-21957][SQL] Add current_user function#19169
mgaido91 wants to merge 1 commit into
apache:masterfrom
mgaido91:SPARK-21957

Conversation

@mgaido91

@mgaido91 mgaido91 commented Sep 8, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Added the current_user function.

How was this patch tested?

Added UTs.

@mgaido91 mgaido91 changed the title [SPARK-21957] Add current_user function [SPARK-21957][SQL] Add current_user function Sep 8, 2017
@holdenk

holdenk commented Sep 8, 2017

Copy link
Copy Markdown
Contributor

The Python side looks reasonable, maybe @marmbrus can share his view on adding this function in general?
In the mean time Jenkins OK to test.

@HyukjinKwon

HyukjinKwon commented Sep 9, 2017

Copy link
Copy Markdown
Member

looks something is gone wrong with Jenkins command .. not sure why but looks we should just say "ok to test"?

@HyukjinKwon

HyukjinKwon commented Sep 9, 2017

Copy link
Copy Markdown
Member

ok to test

gatorsmile
gatorsmile reviewed Sep 9, 2017
View reviewed changes
Comment thread sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/CurrentUser.scala
*/
@ExpressionDescription(
usage = """
_FUNC_() - Returns the current user executing the query.

@gatorsmile gatorsmile Sep 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I do not think Spark has something like SESSION_USER. Thus, this impl is wrong.

gatorsmile
gatorsmile reviewed Sep 9, 2017
View reviewed changes
Comment thread sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/CurrentUser.scala

/** Returns the result of evaluating this expression on a given input Row */
override def eval(input: InternalRow): Any = {
SparkContext.getActive.map(_.sparkUser).getOrElse("")

@gatorsmile gatorsmile Sep 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is from Spark Context. All the sessions share the same user names. I am afraid this value is misleading to our end users. cc @rxin

@mgaido91 mgaido91 Sep 9, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is actually the user which executes the query. I agree that may be misleading that a user can connect to STS with his credentials and then the queries are run with a superuser, but this is how Spark works at the moment and the user which actually runs the query is the superuser. If we don't think to STS, in the other use cases the user which runs a query is the user who started the session.

@gatorsmile gatorsmile Sep 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry, I do not think we should support it until we provide session-specific user management.

@mgaido91 mgaido91 Sep 11, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok, thanks. Then is there any plan to support session-specific user management? If so, can I help somehow? Thanks.

@gatorsmile gatorsmile Oct 23, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So far, it is not in our plan.

@SparkQA

SparkQA commented Sep 9, 2017

Copy link
Copy Markdown

Test build #81580 has finished for PR 19169 at commit 6b0b6bd.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class CurrentUser() extends LeafExpression with CodegenFallback

@SparkQA

SparkQA commented Sep 9, 2017

Copy link
Copy Markdown

Test build #81581 has finished for PR 19169 at commit 6b0b6bd.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class CurrentUser() extends LeafExpression with CodegenFallback

@mgaido91

mgaido91 commented Sep 9, 2017

Copy link
Copy Markdown
Contributor Author

the error the build job is failing for is:

core/src/main/scala/org/apache/spark/io/CompressionCodec.scala:24: imported LZ4BlockInputStream' is permanently hidden by definition of object LZ4BlockInputStream in package io
[warn] import net.jpountz.lz4.{LZ4BlockInputStream, LZ4BlockOutputStream}
[warn]                         ^
[error] 
core/src/main/scala/org/apache/spark/io/CompressionCodec.scala:120: type mismatch;
[error]  found   : Boolean
[error]  required: net.jpountz.lz4.LZ4FastDecompressor
[error]     new LZ4BlockInputStream(s, disableConcatenationOfByteStream)
[error]                                ^
[info] Boolean <: net.jpountz.lz4.LZ4FastDecompressor?
[info] false
[warn] one warning found
[error] one error found
[error] (core/compile:compileIncremental) Compilation failed

this is not to be related to this PR. I am checking the reason which introduced the problem. Any idea?
Thanks.

@HyukjinKwon

HyukjinKwon commented Sep 9, 2017

Copy link
Copy Markdown
Member

The test failure with -9 in Jenkins is unknown up to my knowledge (although there have been few clues and assumptions so far). It should be not related with it.

@maropu

maropu commented Sep 11, 2017

Copy link
Copy Markdown
Member

It seems the failure is caused by the version mismatch v1.3 v.s. v1.4 in lz4-java (recently, I updated in #18883). I'm not sure this happens in jenkins though...

wangyum
wangyum reviewed Sep 11, 2017
View reviewed changes
Comment thread sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/CurrentUser.scala
@@ -0,0 +1,47 @@
/*

@wangyum wangyum Sep 11, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add this class to https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala?

@mgaido91

mgaido91 commented Sep 11, 2017

Copy link
Copy Markdown
Contributor Author

On local I solved the build error by running a mvn clean. As pointed out by @maropu , a PR removed the class and then the incremental compilation fails. I am not sure why this is happening on jenkins though...

@gatorsmile

gatorsmile commented Oct 23, 2017

Copy link
Copy Markdown
Member

So far, session-specific user management is not part of our plan yet. This API is not useful before it. Could you please close this PR?

@mgaido91

mgaido91 commented Oct 24, 2017

Copy link
Copy Markdown
Contributor Author

I think it could have been useful anyway (eg. a user inserting rows through STS who wants to store the name of the user which is connecting to STS and performing the operation), but I agree that these are very corner cases. I am closing this then. Thanks.

@mgaido91 mgaido91 closed this Oct 24, 2017
@dmoore247

dmoore247 commented Apr 1, 2020

Copy link
Copy Markdown

It would be great to see this revived. A lot of old school ETL uses a current_user() function like this for audit/lineage/provenance fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wangyum wangyum wangyum left review comments
@gatorsmile gatorsmile gatorsmile left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@mgaido91 @holdenk @HyukjinKwon @SparkQA @maropu @gatorsmile @dmoore247 @wangyum