From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:2:c151::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id mDw5LqM3UGCjbAAA0tVLHw (envelope-from ) for ; Tue, 16 Mar 2021 04:44:19 +0000 Received: from aspmx2.migadu.com ([2001:41d0:2:c151::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id cHosKqM3UGC+CgAA1q6Kng (envelope-from ) for ; Tue, 16 Mar 2021 04:44:19 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx2.migadu.com (Postfix) with ESMTPS id D474027714 for ; Tue, 16 Mar 2021 05:44:18 +0100 (CET) Received: from localhost ([::1]:54016 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lM1Z3-0004m2-Rk for larch@yhetil.org; Tue, 16 Mar 2021 00:44:17 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:59226) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lM1YP-0004ld-Ij for emacs-orgmode@gnu.org; Tue, 16 Mar 2021 00:43:37 -0400 Received: from out0.migadu.com ([2001:41d0:2:267::]:46201) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lM1YM-0004Sk-Pe for emacs-orgmode@gnu.org; Tue, 16 Mar 2021 00:43:37 -0400 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kyleam.com; s=key1; t=1615869810; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=/4AaQTCWepWVGgKt6RcrEcdajUXXQCg5QqeP8y9MD84=; b=fJNz3RO662tQ8SZt57Rm4/bCHrRDiFKmiCSXXMJAGNFhGdrrZOxpFP4Xodnj0wDAC4DsPn Y1X8QS1c77woExEiFqBJh5s3pA1PH1I4LBjW8B0L8gcT3R7GH8XHAP8IzQY2uJZAG1DRpT hXN7K8zr9NIFK3DUwawk588b1+oWYb0nd3a5CXFJTlTLJMzrcl20ufLSRBcQiL3iXAbo1Z oLBTYTPywRaP3CCu2LwG1PclLi2DJJa6y02VkTtlg4B93b69tXAlCa2uNaZlM9k1Eiqqw3 x9gVHHZTvGDBb+JcGf7fZOv8liapWdFPggDzTs8CPqtd3+DQ2Zr2qpcsZZKXEQ== From: Kyle Meyer To: Robin Campbell Joy Subject: Re: [PATCH] ob-sql.el: Add support for SAP HANA In-Reply-To: References: Date: Tue, 16 Mar 2021 00:43:23 -0400 Message-ID: <87lfaniu8k.fsf@kyleam.com> MIME-Version: 1.0 Content-Type: text/plain X-Migadu-Auth-User: kyle@kyleam.com Received-SPF: pass client-ip=2001:41d0:2:267::; envelope-from=kyle@kyleam.com; helo=out0.migadu.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: emacs-orgmode@gnu.org Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1615869859; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=/4AaQTCWepWVGgKt6RcrEcdajUXXQCg5QqeP8y9MD84=; b=bky6nqqR2Obk36oOs+0juPt/f5JIh4ImLLKJKThITK8TIMmGjMWcFkSNgwdxTh5XIiY0Yt bkFPiWigHqq0QySJCZcosTRpOtAc09i/EqZOnCOGN3ZVarQZF/SCvG2wTziCEsuhBJL6kz uFkbuesy9PQOyWI9QOavYU7ETzJnZFZ+UZYoHs+K4ecIVKgGjN3eUKytoPfsDMJbwspfsG R/usH0ipbHBSfWIgDZAkkJAuJeU1bSCzRu30RA+3CCmjJ4+iT0pYuVBH/KHnbAZDYdAWf/ zf37PF4IFVqZv1UwmTWzzBS9ajjYuKfFN5t1HZcVcuRHuU5l1sGIleb7zRq/9w== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1615869859; a=rsa-sha256; cv=none; b=rYIaHrmvhknAmk60LlgdISemhyDtnzXPghw6gCdTVHNwU3U+v5giK4YJnt2SRRf9LGYHoS lPbrTPP/mcDntP/YxdM633PqYAah1PnIyeC+yP7V/L5yzhewrakwyhkStPyJTSNbk8tYwm 9EGVfOI4JgaNwCr1A48oomzKkrqyF5WWbZ1fKEj+XTzWLLqjEFo32vFkTTRUsug9NkdI3q vfjNN5eKOvkE539e5T+tTkb4Et45FGE+mJCo3pntXUBfyjVL9e52rT/2hdfr8r91AshZIm NotrUCusdZZ8yhkgBmZrZMQ3+5V7Lidy8Nbf6+aUdd5U3QKBgBTj/N/QS4n0ng== ARC-Authentication-Results: i=1; aspmx2.migadu.com; dkim=pass header.d=kyleam.com header.s=key1 header.b=fJNz3RO6; spf=pass (aspmx2.migadu.com: domain of emacs-orgmode-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=emacs-orgmode-bounces@gnu.org X-Migadu-Spam-Score: -2.60 Authentication-Results: aspmx2.migadu.com; dkim=pass header.d=kyleam.com header.s=key1 header.b=fJNz3RO6; dmarc=none; spf=pass (aspmx2.migadu.com: domain of emacs-orgmode-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=emacs-orgmode-bounces@gnu.org X-Migadu-Queue-Id: D474027714 X-Spam-Score: -2.60 X-Migadu-Scanner: scn0.migadu.com X-TUID: SkvWWGbfXtDw Robin Campbell Joy writes: > Hi, > > could someone please let me know if something is missing/incorrect? Thanks for the patch, and sorry for the delay. > On Thu, 4 Feb 2021 at 08:55, Robin Campbell Joy wrote: > >> * lisp/ob-sql.el (org-babel-execute:sql, org-babel-sql-dbstring-saphana): >> Add basic support for SAP HANA to SQL blocks >> * testing/lisp/test-ob-sql.el: Basic tests for generated db connection >> string Style nit: missing periods at the end of the changelog entries. https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html#Style-of-Change-Logs >> This change adds basic support for SAP HANA to SQL blocks by >> specifying saphana as :engine. >> >> It also adds a new header arg `dbinstance' in order to specify the SAP >> HANA instance to connect to. >> >> Signed-off-by: Robin Campbell Joy This trailer is fine but doesn't have any meaning in this project. This patch is large enough that copyright should be assigned to the FSF. Assuming you haven't already, are you willing to complete the copyright paperwork? https://orgmode.org/worg/org-contribute.html#copyright-issues >> --- >> lisp/ob-sql.el | 25 ++- >> testing/lisp/test-ob-sql.el | 382 ++++++++++++++++++++++++++++++++++++ Great, thanks for adding tests. >> 2 files changed, 406 insertions(+), 1 deletion(-) >> create mode 100644 testing/lisp/test-ob-sql.el >> >> diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el The patch doesn't apply. $ curl -fSs \ https://orgmode.org/list/CADzxVkEO=X6r_YAi3qjKOoJiPVPHxpcFRc2jAW7fpUFs92w3Kg@mail.gmail.com/raw | \ git am Applying: ob-sql.el: Add support for SAP HANA error: corrupt patch at line 40 Patch failed at 0001 ob-sql.el: Add support for SAP HANA [...] It looks like many of the lines are corrupted by additional line breaks, so it'd take a lot of manual editing to resolve the issues on my end. >From scanning through it, the patch looks well done. Here are a few quick comments. >> index 902194ae8..5398c85aa 100644 >> --- a/lisp/ob-sql.el >> +++ b/lisp/ob-sql.el >> @@ -40,6 +40,7 @@ >> ;; - dbuser >> ;; - dbpassword >> ;; - dbconnection (to reference connections in sql-connection-alist) >> +;; - dbinstance Hmm, so if we can't shoehorn this into an existing parameter, I guess this should mention that dbinstance is relevant only for saphana? >> ;; - database >> ;; - colnames (default, nil, means "yes") >> ;; - result-params >> @@ -58,6 +59,7 @@ >> ;; - postgresql (postgres) >> ;; - oracle >> ;; - vertica >> +;; - saphana >> ;; >> ;; TODO: >> ;; >> @@ -85,6 +87,7 @@ >> (dbport . :any) >> (dbuser . :any) >> (dbpassword . :any) >> + (dbinstance . :any) >> (database . :any)) >> "SQL-specific header arguments.") >> >> @@ -174,6 +177,18 @@ SQL Server on Windows and Linux platform." >> (when database (format "-d %s" database)))) >> " ")) >> >> +(defun org-babel-sql-dbstring-saphana (host port instance user password >> database) >> + "Make SAP HANA command line args for database connection. Pass nil to >> omit that arg." To follow the Emacs docstring convention, "Pass" should be moved to the second line. >> + (mapconcat #'identity >> + (delq nil >> + (list (when (and host port) (format "-n %s:%s" host >> port)) Please prefer `and' here and in other spots where the return value is of interest. (and host port (format ...)) (Possibly breaking it up across lines if you're getting over ~80 chars.)