Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 1B1E0200BAC for ; Wed, 26 Oct 2016 22:00:53 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 19989160AEE; Wed, 26 Oct 2016 20:00:53 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 11E36160AE1 for ; Wed, 26 Oct 2016 22:00:51 +0200 (CEST) Received: (qmail 20165 invoked by uid 500); 26 Oct 2016 20:00:50 -0000 Mailing-List: contact dev-help@spark.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list dev@spark.apache.org Received: (qmail 20150 invoked by uid 99); 26 Oct 2016 20:00:50 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd3-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 26 Oct 2016 20:00:50 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd3-us-west.apache.org (ASF Mail Server at spamd3-us-west.apache.org) with ESMTP id E641318068F for ; Wed, 26 Oct 2016 20:00:49 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd3-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 2.48 X-Spam-Level: ** X-Spam-Status: No, score=2.48 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=2, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RCVD_IN_SORBS_SPAM=0.5] autolearn=disabled Authentication-Results: spamd3-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=tresata-com.20150623.gappssmtp.com Received: from mx1-lw-eu.apache.org ([10.40.0.8]) by localhost (spamd3-us-west.apache.org [10.40.0.10]) (amavisd-new, port 10024) with ESMTP id vKbN4WIdgBl5 for ; Wed, 26 Oct 2016 20:00:46 +0000 (UTC) Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com [74.125.82.44]) by mx1-lw-eu.apache.org (ASF Mail Server at mx1-lw-eu.apache.org) with ESMTPS id 009925F240 for ; Wed, 26 Oct 2016 20:00:45 +0000 (UTC) Received: by mail-wm0-f44.google.com with SMTP id e69so54996575wmg.0 for ; Wed, 26 Oct 2016 13:00:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tresata-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=Gwc5PBRPpeReFHbLTIYGz0hLDx7tqEY6Jiv+XjlWDGU=; b=kUKUUc0uSpuq0OXxFFWzntkCVrX4dkIrgc859hYq+3oyJloKs4bhIYchSQA94MGEQc eh0c2ec7dcrM4DIevS1Gx0HXuz6+pBX7fjjEW3MdJNsx89t4jlT0N9H12MDHZDiWRYae 8GMj/H2mLBHXAYHpMwlrg7LLIp34S/ICOgnzLkjS+Y9dRdjw5LLFiwC0PlRXywUaOnwz nI7BSUZkoFqAxTKemg11PD+xZCh3QdrZxRjuXpZ2jfHhiWYxT9e+W9IkUaLVBzwiQO2q OIxuvmIJmxnREhcln30qp1DadB6H9zcbiEnVrmemOZmyY/9hE39KJeBCaYnESlwCVcYa nxpg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Gwc5PBRPpeReFHbLTIYGz0hLDx7tqEY6Jiv+XjlWDGU=; b=RuLzUF3qiSnhmD1bn0b78MlLc6cuawo0G6KtdnVwdvR/Fb/3GXSaWyHfzfNL9YZckh 7TQo8I0MrynLQEqaADyueV4/FIeCk5ydnhGTmdfuQ7Y0asaQMnfo8/aoXJr/NU26m+iU JSR++hiYBNZ5dBYnOT1ATFbNYBFW7d2ihNMpm2Yq6cPFi1z+Dc7Oo/T4OpcHUO5j5gnw I5x2cnTl45sSLoWudzX5sBcEUDu32R/J5gJqDtAgyvrOr3T8XoaZIDsx3D1ADL3j7NbL YAQmEwGRct+0/ZWTJgkFsi/rqgNmjJo5oZ+1rEa3tlPRuXhwgDBuwFdg9N1HsH8Bf5k/ MX2Q== X-Gm-Message-State: ABUngvezt8rtAi5qd2mRCy8BQd8zD8Ri71ESInf8PQEuzEWj87evkB7ZZ6kZ41LIf88eftaVH9/4WEZtnAxVtQ== X-Received: by 10.28.26.75 with SMTP id a72mr4183007wma.36.1477512045338; Wed, 26 Oct 2016 13:00:45 -0700 (PDT) MIME-Version: 1.0 Received: by 10.195.30.10 with HTTP; Wed, 26 Oct 2016 13:00:44 -0700 (PDT) X-Originating-IP: [209.150.50.93] In-Reply-To: References: From: Koert Kuipers Date: Wed, 26 Oct 2016 16:00:44 -0400 Message-ID: Subject: Re: getting encoder implicits to be more accurate To: Michael Armbrust Cc: Ryan Blue , "dev@spark.apache.org" Content-Type: multipart/alternative; boundary=001a114cd8be9d53a3053fca1482 archived-at: Wed, 26 Oct 2016 20:00:53 -0000 --001a114cd8be9d53a3053fca1482 Content-Type: text/plain; charset=UTF-8 why would generating implicits for ProductN where you also require the elements in the Product to have an expression encoder not work? we do this. and then we have a generic fallback where it produces a kryo encoder. for us the result is that say an implicit for Seq[(Int, Seq[(String, Int)])] will create a new ExpressionEncoder(), while an implicit for Seq[(Int, Set[(String, Int)])] produces a Encoders.kryoEncoder() On Wed, Oct 26, 2016 at 3:50 PM, Michael Armbrust wrote: > Sorry, I realize that set is only one example here, but I don't think that > making the type of the implicit more narrow to include only ProductN or > something eliminates the issue. Even with that change, we will fail to > generate an encoder with the same error if you, for example, have a field > of your case class that is an unsupported type. > > Short of changing this to compile-time macros, I think we are stuck with > this class of errors at runtime. The simplest solution seems to be to > expand the set of thing we can handle as much as possible and allow users > to turn on a kryo fallback for expression encoders. I'd be hesitant to > make this the default though, as behavior would change with each release > that adds support for more types. I would be very supportive of making > this fallback a built-in option though. > > On Wed, Oct 26, 2016 at 11:47 AM, Koert Kuipers wrote: > >> yup, it doesnt really solve the underlying issue. >> >> we fixed it internally by having our own typeclass that produces encoders >> and that does check the contents of the products, but we did this by simply >> supporting Tuple1 - Tuple22 and Option explicitly, and not supporting >> Product, since we dont have a need for case classes >> >> if case classes extended ProductN (which they will i think in scala >> 2.12?) then we could drop Product and support Product1 - Product22 and >> Option explicitly while checking the classes they contain. that would be >> the cleanest. >> >> >> On Wed, Oct 26, 2016 at 2:33 PM, Ryan Blue wrote: >> >>> Isn't the problem that Option is a Product and the class it contains >>> isn't checked? Adding support for Set fixes the example, but the problem >>> would happen with any class there isn't an encoder for, right? >>> >>> On Wed, Oct 26, 2016 at 11:18 AM, Michael Armbrust < >>> michael@databricks.com> wrote: >>> >>>> Hmm, that is unfortunate. Maybe the best solution is to add support >>>> for sets? I don't think that would be super hard. >>>> >>>> On Tue, Oct 25, 2016 at 8:52 PM, Koert Kuipers >>>> wrote: >>>> >>>>> i am trying to use encoders as a typeclass where if it fails to find >>>>> an ExpressionEncoder it falls back to KryoEncoder. >>>>> >>>>> the issue seems to be that ExpressionEncoder claims a little more than >>>>> it can handle here: >>>>> implicit def newProductEncoder[T <: Product : TypeTag]: Encoder[T] = >>>>> Encoders.product[T] >>>>> >>>>> this "claims" to handle for example Option[Set[Int]], but it really >>>>> cannot handle Set so it leads to a runtime exception. >>>>> >>>>> would it be useful to make this a little more specific? i guess the >>>>> challenge is going to be case classes which unfortunately dont extend >>>>> Product1, Product2, etc. >>>>> >>>> >>>> >>> >>> >>> -- >>> Ryan Blue >>> Software Engineer >>> Netflix >>> >> >> > --001a114cd8be9d53a3053fca1482 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
why would generating implicits for ProductN where you also r= equire the elements in the Product to have an expression encoder not work?<= br>
we do this. and then we have a generic fallback where it produce= s a kryo encoder.

for us the result is that say an implicit for= Seq[(Int, Seq[(String, Int)])] will create a new ExpressionEncoder(), whil= e an implicit for Seq[(Int, Set[(String, Int)])] produces a Encoders.kryoEn= coder()

On Wed, Oct 26, 2016 at 3:50 PM, Michael Armbrust &= lt;michael@data= bricks.com> wrote:
Sorry, I realize that set is only one example here, but I don'= t think that making the type of the implicit more narrow to include only Pr= oductN or something eliminates the issue.=C2=A0 Even with that change, we w= ill fail to generate an encoder with the same error if you, for example, ha= ve a field of your case class that is an unsupported type.

Short of changing this to compile-time macros, I think we are stuck with= this class of errors at runtime.=C2=A0 The simplest solution seems to be t= o expand the set of thing we can handle as much as possible and allow users= to turn on a kryo fallback for expression encoders.=C2=A0 I'd be hesit= ant to make this the default though, as behavior would change with each rel= ease that adds support for more types.=C2=A0 I would be very supportive of = making this fallback a built-in option though.

On Wed, Oct 26, 2016 at 11:47 AM, Koert Kuipers <= ;koert@tresata.com> wrote:
yup, = it doesnt really solve the underlying issue.

we fixed it interna= lly by having our own typeclass that produces encoders and that does check = the contents of the products, but we did this by simply supporting Tuple1 -= Tuple22 and Option explicitly, and not supporting Product, since we dont h= ave a need for case classes

if case classes extended ProductN (w= hich they will i think in scala 2.12?) then we could drop Product and suppo= rt Product1 - Product22 and Option explicitly while checking the classes th= ey contain. that would be the cleanest.


On Wed, Oct 26, 2016 at 2:= 33 PM, Ryan Blue <rblue@netflix.com> wrote:
Isn't the problem that Option is a P= roduct and the class it contains isn't checked? Adding support for Set = fixes the example, but the problem would happen with any class there isn= 9;t an encoder for, right?

On Wed, Oct 26, 2016 at 11:18 AM, Michael Armbrust <michael@da= tabricks.com> wrote:
Hmm, that is unfortunate.=C2=A0 Maybe the best solution is to ad= d support for sets?=C2=A0 I don't think that would be super hard.
=

On Tue, Oct 25, 2016 at 8:52 PM, Koert Kuipers <koert@tresata.com&g= t; wrote:
i am tryi= ng to use encoders as a typeclass where if it fails to find an ExpressionEn= coder it falls back to KryoEncoder.

the issue seems to be that E= xpressionEncoder claims a little more than it can handle here:
=C2=A0 im= plicit def newProductEncoder[T <: Product : TypeTag]: Encoder[T] =3D Enc= oders.product[T]

this "claims" to handle for example O= ption[Set[Int]], but it really cannot handle Set so it leads to a runtime e= xception.

would it be useful to make this a little more specific= ? i guess the challenge is going to be case classes which unfortunately don= t extend Product1, Product2, etc.




<= /div>--
Ryan Blue
Software Engin= eer
Netflix



--001a114cd8be9d53a3053fca1482--