14/f4d0faaedb51c7afd590d7ebf4267a6d59c17b

   1 Return-Path: <amdragon@mit.edu>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 9D5AD431FAF\r
   6         for <notmuch@notmuchmail.org>; Tue, 25 Dec 2012 17:23:07 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id m+ueMU2URmHt for <notmuch@notmuchmail.org>;\r
  16         Tue, 25 Dec 2012 17:23:07 -0800 (PST)\r
  17 Received: from dmz-mailsec-scanner-3.mit.edu (DMZ-MAILSEC-SCANNER-3.MIT.EDU\r
  18         [18.9.25.14])\r
  19         by olra.theworths.org (Postfix) with ESMTP id DC466431FAE\r
  20         for <notmuch@notmuchmail.org>; Tue, 25 Dec 2012 17:23:06 -0800 (PST)\r
  21 X-AuditID: 1209190e-b7fa16d000001402-81-50da51791370\r
  22 Received: from mailhub-auth-3.mit.edu ( [18.9.21.43])\r
  23         by dmz-mailsec-scanner-3.mit.edu (Symantec Messaging Gateway) with SMTP\r
  24         id 5E.0D.05122.9715AD05; Tue, 25 Dec 2012 20:23:05 -0500 (EST)\r
  25 Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
  26         by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id qBQ1N43n016953; \r
  27         Tue, 25 Dec 2012 20:23:04 -0500\r
  28 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  29         (authenticated bits=0)\r
  30         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  31         by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id qBQ1N1HD002404\r
  32         (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
  33         Tue, 25 Dec 2012 20:23:02 -0500 (EST)\r
  34 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
  35         (envelope-from <amdragon@MIT.EDU>)\r
  36         id 1Tnfi0-0008Ut-Qk; Tue, 25 Dec 2012 20:23:00 -0500\r
  37 Date: Tue, 25 Dec 2012 20:23:00 -0500\r
  38 From: Austin Clements <amdragon@MIT.EDU>\r
  39 To: David Bremner <david@tethera.net>\r
  40 Subject: Re: [PATCH 2/5] util: Function to parse boolean term queries\r
  41 Message-ID: <20121226012300.GW6187@mit.edu>\r
  42 References: <1356415076-5692-1-git-send-email-amdragon@mit.edu>\r
  43         <1356415076-5692-3-git-send-email-amdragon@mit.edu>\r
  44         <87obhidxkt.fsf@zancas.localnet>\r
  45 MIME-Version: 1.0\r
  46 Content-Type: text/plain; charset=us-ascii\r
  47 Content-Disposition: inline\r
  48 In-Reply-To: <87obhidxkt.fsf@zancas.localnet>\r
  49 User-Agent: Mutt/1.5.21 (2010-09-15)\r
  50 X-Brightmail-Tracker:\r
  51  H4sIAAAAAAAAA+NgFprOKsWRmVeSWpSXmKPExsUixCmqrVsZeCvAYMNMFYsbrd2MFk3TnS1W\r
  52         z+WxuH5zJrMDi8fOWXfZPW7df83u8WzVLWaPLYfeMwewRHHZpKTmZJalFunbJXBldHxbwFbQ\r
  53         KVzx/vZa1gbGDr4uRk4OCQETibZfTUwQtpjEhXvr2boYuTiEBPYxSnzpnMgO4WxglLi1qAPK\r
  54         ucgk0bPpOzOEs4RRYtKdh2D9LAKqEjdnvWcEsdkENCS27V8OZosAxa9um8wGYjMLuEusn3gG\r
  55         rF5YwFXifmMvWA2vgLbE5+tTGSGGzmCUePRxFTtEQlDi5MwnLBDNWhI3/r0EauYAsqUllv/j\r
  56         AAlzCuhKNP4+BzZTVEBFYsrJbWwTGIVmIemehaR7FkL3AkbmVYyyKblVurmJmTnFqcm6xcmJ\r
  57         eXmpRbrGermZJXqpKaWbGMHhL8m3g/HrQaVDjAIcjEo8vBu/3wwQYk0sK67MPcQoycGkJMp7\r
  58         3v9WgBBfUn5KZUZicUZ8UWlOavEhRgkOZiURXuePQOW8KYmVValF+TApaQ4WJXHeKyk3/YUE\r
  59         0hNLUrNTUwtSi2CyMhwcShK8cwOAhgoWpaanVqRl5pQgpJk4OEGG8wANnwpSw1tckJhbnJkO\r
  60         kT/FqCglzpsLkhAASWSU5sH1wtLTK0ZxoFeEeZeBVPEAUxtc9yugwUxAg2P5boAMLklESEk1\r
  61         ME73DVxwvsr5by/vJsFPWXd/yM+ePuHeuYKlrpPnqZTZSd9dZKuRz5JzMe/5JtEQv+X8eZzb\r
  62         lCYb6+4yrtjJpp9wrpXhjWzMxbK38VtX3zrCkvtisl3prO8TXqfpBN3eKlpt6lFXOpNr6tag\r
  63         d/H+a39N/N9wfQHLUXOjC2d9a+3cbQ6pWM0WdVNiKc5INNRiLipOBAAJELYQKgMAAA==\r
  64 Cc: notmuch@notmuchmail.org\r
  65 X-BeenThere: notmuch@notmuchmail.org\r
  66 X-Mailman-Version: 2.1.13\r
  67 Precedence: list\r
  68 List-Id: "Use and development of the notmuch mail system."\r
  69         <notmuch.notmuchmail.org>\r
  70 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  71         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  72 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  73 List-Post: <mailto:notmuch@notmuchmail.org>\r
  74 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  75 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  76         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  77 X-List-Received-Date: Wed, 26 Dec 2012 01:23:07 -0000\r
  78 \r
  79 Quoth David Bremner on Dec 25 at 10:18 am:\r
  80 > Austin Clements <amdragon@MIT.EDU> writes:\r
  81 > \r
  82 > > +    if (consume_double_quote (&pos)) {\r
  83 > > +   char *out = talloc_strdup (ctx, pos);\r
  84 > > +   pos = *term_out = out;\r
  85 > > +   while (1) {\r
  86 > \r
  87 > Overall the control flow here is a bit tricky to follow. I'm not sure if\r
  88 > a real loop condition would help or make it worse.\r
  89 > \r
  90 > > +       if (! *pos) {\r
  91 > > +           /* Premature end of string */\r
  92 > > +           goto FAIL;\r
  93 > > +       } else if (*pos == '"') {\r
  94 > > +           if (*++pos != '"')\r
  95 > > +               break;\r
  96 > > +       } else if (consume_double_quote (&pos)) {\r
  97 > > +           break;\r
  98 > > +       }\r
  99 > \r
 100 > I'm confused by the asymmetry here. Quoted strings can start with\r
 101 > unicode quotes, but internally can only have ascii '"'? Is this\r
 102 > documented somewhere?\r
 103 \r
 104 Only in the source, to my knowledge.  Here's how Xapian parses these\r
 105 things (where 'it' is a UTF8 string iterator):\r
 106 \r
 107 if (it != end && is_double_quote(*it)) {\r
 108     // Quoted boolean term (can contain any character).\r
 109     ++it;\r
 110     while (it != end) {\r
 111         if (*it == '"') {\r
 112             // Interpret "" as an escaped ".\r
 113             if (++it == end || *it != '"')\r
 114                 break;\r
 115         } else if (is_double_quote(*it)) {\r
 116             ++it;\r
 117             break;\r
 118         }\r
 119         Unicode::append_utf8(name, *it++);\r
 120     }\r
 121 } else {\r
 122     // Can't boolean filter prefix a subexpression, so\r
 123     // just use anything following the prefix until the\r
 124     // next space or ')' as part of the boolean filter\r
 125     // term.\r
 126     while (it != end && *it > ' ' && *it != ')')\r
 127         Unicode::append_utf8(name, *it++);\r
 128 }\r
 129 \r
 130 > > +    } else {\r
 131 > > +   while (*pos > ' ' && *pos != ')')\r
 132 > > +       ++pos;\r
 133 > > +   if (*pos)\r
 134 > > +       goto FAIL;\r
 135 > > +    }\r
 136 > \r
 137 > So if there is no quote, we skip the part after the ':'? I guess I\r
 138 > probably missed something because that doesn't sound like the intended\r
 139 > behaviour.\r
 140 \r
 141 This isn't skipping it; it's checking its well-formedness.  In this\r
 142 case, *term_out already points to a correct string that can be used\r
 143 literally; we just have to check that there's no trailing garbage\r
 144 after the boolean query.\r
 145 \r
 146 This is certainly worth commenting.\r
 147 \r
 148 For the record, I also tried passing the query straight to the\r
 149 library, without parsing it in the CLI (and simply checking that the\r
 150 query returned exactly one result), and it was noticeably slower (the\r
 151 restore performance test took between 3.82 and 5.25 seconds for the\r
 152 code in this series and ~7.2 seconds using a general query.)\r