Jacek,

This is definitely not necessary, but I wouldn't waste cycles "fixing"
things like this when they have virtually zero impact. Perhaps next time we
update this code we can "fix" it.

Also can you comment on the pull request directly?


On Tue, Jul 5, 2016 at 1:07 PM, Jacek Laskowski <ja...@japila.pl> wrote:

> On Mon, Jul 4, 2016 at 6:14 AM,  <wenc...@apache.org> wrote:
> > Repository: spark
> > Updated Branches:
> >   refs/heads/master 88134e736 -> 8cdb81fa8
> >
> >
> > [SPARK-15204][SQL] improve nullability inference for Aggregator
> >
> > ## What changes were proposed in this pull request?
> >
> > TypedAggregateExpression sets nullable based on the schema of the
> outputEncoder
> >
> > ## How was this patch tested?
> >
> > Add test in DatasetAggregatorSuite
> >
> > Author: Koert Kuipers <ko...@tresata.com>
> ...
> > +    assert(ds1.select(typed.sum((i: Int) => i)).schema.head.nullable
> === false)
> > +    val ds2 = Seq(AggData(1, "a"), AggData(2, "a")).toDS()
> > +    assert(ds2.select(SeqAgg.toColumn).schema.head.nullable === true)
> > +    val ds3 = sql("SELECT 'Some String' AS b, 1279869254 AS
> a").as[AggData]
> > +    assert(ds3.select(NameAgg.toColumn).schema.head.nullable === true)
>
> Why do we assert predicates? If it's true, it's true already (no need
> to compare whether it's true or not). I'd vote to "fix" it.
>
> Jacek
>
> ---------------------------------------------------------------------
> To unsubscribe e-mail: dev-unsubscr...@spark.apache.org
>
>

Reply via email to