removing most of the config functions in SQLConf?

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

removing most of the config functions in SQLConf?

rxin
In SQLConf, for each config option, we declare them in two places:

First in the SQLConf object, e.g.:
val CSV_PARSER_COLUMN_PRUNING = buildConf("spark.sql.csv.parser.columnPruning.enabled")
.internal()
.doc("If it is set to true, column names of the requested schema are passed to CSV parser. " +
"Other column values can be ignored during parsing even if they are malformed.")
.booleanConf
.createWithDefault(true)

Second in SQLConf class:

def csvColumnPruning: Boolean = getConf(SQLConf.CSV_PARSER_COLUMN_PRUNING)


As the person that introduced both, I'm now thinking we should remove almost all of the latter, unless it is used more than 5 times. The vast majority of config options are read only in one place, so the functions are pretty redundant ...




Reply | Threaded
Open this post in threaded view
|

Re: removing most of the config functions in SQLConf?

cloud0fan
IIRC, the reason we did it is: `SQLConf` was in SQL core module. So we need to create methods in `CatalystConf`, and `SQLConf` implements `CatalystConf`.

Now the problem has gone: we moved `SQLConf` to catalyst module. I think we can remove these methods.

On Fri, Dec 14, 2018 at 3:45 PM Reynold Xin <[hidden email]> wrote:
In SQLConf, for each config option, we declare them in two places:

First in the SQLConf object, e.g.:
val CSV_PARSER_COLUMN_PRUNING = buildConf("spark.sql.csv.parser.columnPruning.enabled")
.internal()
.doc("If it is set to true, column names of the requested schema are passed to CSV parser. " +
"Other column values can be ignored during parsing even if they are malformed.")
.booleanConf
.createWithDefault(true)

Second in SQLConf class:

def csvColumnPruning: Boolean = getConf(SQLConf.CSV_PARSER_COLUMN_PRUNING)


As the person that introduced both, I'm now thinking we should remove almost all of the latter, unless it is used more than 5 times. The vast majority of config options are read only in one place, so the functions are pretty redundant ...




Reply | Threaded
Open this post in threaded view
|

Re: removing most of the config functions in SQLConf?

Darcy Shen-2

I agree with the CatalystConf idea.




---- On Fri, 14 Dec 2018 18:40:26 +0800 Wenchen Fan<[hidden email]> wrote ----

IIRC, the reason we did it is: `SQLConf` was in SQL core module. So we need to create methods in `CatalystConf`, and `SQLConf` implements `CatalystConf`.

Now the problem has gone: we moved `SQLConf` to catalyst module. I think we can remove these methods.

On Fri, Dec 14, 2018 at 3:45 PM Reynold Xin <[hidden email]> wrote:
In SQLConf, for each config option, we declare them in two places:

First in the SQLConf object, e.g.:
val CSV_PARSER_COLUMN_PRUNING = buildConf("spark.sql.csv.parser.columnPruning.enabled")
.internal()
.doc("If it is set to true, column names of the requested schema are passed to CSV parser. " +
"Other column values can be ignored during parsing even if they are malformed.")
.booleanConf
.createWithDefault(true)

Second in SQLConf class:

def csvColumnPruning: Boolean = getConf(SQLConf.CSV_PARSER_COLUMN_PRUNING)


As the person that introduced both, I'm now thinking we should remove almost all of the latter, unless it is used more than 5 times. The vast majority of config options are read only in one place, so the functions are pretty redundant ...