Some of you probably wondered where the heck the recent flurry of activity
around regular expressions (eg, commits 9fe8fe9c9e, b63fc2877) came from.
The answer is that it was mostly driven by some fuzz testing that Greg
Stark reported to the PG security list: he found various random regexp
patterns that crashed the regex compiler and/or caused it to take
unreasonable amounts of time and memory.  Some of this was already known;
we had such a report back at
http://www.postgresql.org/message-id/b6f6fd62f2624c4c9916ac0175d56d880ce20...@jenmbs01.ad.intershop.net
and there are various similar complaints in the Tcl bug tracker.

The conclusion of the security list was that we didn't want to treat
such problems as CVE-worthy security issues, because given the history
of that code, no security-conscious DBA would allow use of regexes coming
from untrustworthy sources.  (I'm not 100% on board with that reasoning
personally, though I have to agree that the warning we added about it was
prudent.)  But we did do our best to fix cases where regex compilation
might be uncancelable for long intervals or might recurse to the point
of stack overflow, which were both things Greg showed to be possible.
That left us with some pretty severe performance issues, but hopefully
nothing that use of a statement timeout can't deal with.

Anyway, I got interested in whether the performance issues could be dealt
with properly, and have spent probably more time than I should've with my
nose buried in the regex engine.  I've found a number of things that are
indeed fixable, and the attached set of patches are the result.

The primary thing that we found out is that the patch originally applied
to fix CVE-2007-4772 (infinite loop in pullback) is broken: not only does
it not fix all related cases, but it is capable of causing an Assert
failure.  Fortunately the assertion condition is harmless in production
builds, so we didn't need to treat that as a security issue either.  But
it's still possible to make pullback() go into an infinite loop that will
be broken only by running into the NFA state count limit.

Some background: what the pullback() and pushfwd() routines are trying to
do is get rid of ^ and $ constraint arcs by pushing them to the start and
stop states respectively, or dropping them as unsatisfiable if they
can't be so pushed (for example, in "a^x" there is no possible way to
satisfy the ^ constraint).  Likewise AHEAD and BEHIND constraints can be
pushed to the start and stop states or discarded as either certainly
satisfied or certainly not satisfied.  However, this works only if there
are no loops of constraint arcs, otherwise the transformation just keeps
pushing the same constraint around the loop, generating more and more
states.  The pattern that prompted CVE-2007-4772 was "($|^)*", which
gives rise to an NFA state with ^ and $ constraint arcs that lead to
itself.  So the hack that was instituted to deal with that was just to
drop such arcs as useless, which they are.  (Whether or not you follow
the arc, you end up in the same state you started in, and you don't
consume any input; so it's useless to follow the arc.)  However, that only
fixes trivial single-state constraint loops.  It is not difficult to
construct regexes with constraint loops connecting more than one state.
Here are some examples:
        (^$)*
        (^(?!aa)(?!bb)(?!cc))+
The latter can be extended to construct a loop of as many states as you
please.

Also, I mentioned that the previous patch could cause an assertion
failure; the pattern "(^)+^" causes that.  This happens because the
way the patch was implemented was to remove single-state loop arcs
within pull() or push(), and that subtly breaks the API contract
of those functions.  If the removed loop arc was the final outarc
of its source state, and the only other outarc was another constraint
that had been passed to pull(), then pull() would remove both arcs
and then conclude it could delete the source state as well.  We'd
then come back to pullback() whose nexta variable is pointing at a
now-deleted arc.  Fortunately freearc() sets the type of deleted arcs
to zero, so the loop in pullback() would do nothing with the deleted
arc and exit successfully --- but the assert() that's there would
notice that something was wrong.

I concluded that the only way to fix this was to get rid of the hacks in
pull/push and instead implement a general constraint-loop-breaking pass
before we attempt pullback/pushfwd.

In principle it's okay to break a loop of constraint arcs for the same
reason cited above: traversing all the way around the loop, even if it's
possible because all the constraints are satisfied at the current point of
the string, is pointless since you're back in the same NFA state without
having consumed any input characters.  Therefore we can break the loop by
removing the constraint arc(s) between any two chosen loop states, as long
as we make sure that it's still possible to follow any legal transition
sequence that would have involved less than one full trip around the loop.
This can be implemented by generating clone states that we chain onto the
predecessor state of the removed arc(s).  For example, if we have a loop
involving S1, S2, S3, and we choose to remove the constraint arc from S1
to S2, we make clone states S2C and S3C; S1's former constraint outarc to
S2 now goes to S2C, and S2's constraint outarc to S3 now has a cloned copy
from S2C to S3C, but we don't clone the arc from S3 to S1 so as not to
create a new loop.  All of S2's and S3's non-loop-involved outarcs are
cloned to S2C and S3C so that you can still get to any loop exit states
you could've before.

Getting this right and reasonably performant took a few tries :-(,
but I'm pretty happy with the final result, which is attached as
re-fixconstraintloops.patch.

Once we had the infinite-loop problem taken care of, some of Greg's test
cases that created larger NFAs were still taking unreasonable amounts of
time.  Performance tracing showed that the problem was some algorithms
that involved O(N^2) time for states with N in-arcs or out-arcs.  One
issue is that the lists chaining in-arcs and out-arcs of each state were
singly linked, meaning that freearc() took O(N) time, and deleting all the
arcs of a state in turn could take O(N^2) time if you deleted them in an
inconvenient order, which sometimes happens.  The attached patch fixes
that by making those lists doubly-linked.  (We'd already made the "color
chains" doubly-linked years ago; not sure why we thought we could get away
with keeping single linking for the inchains and outchains.)  Another
problem area was duplicate-arc detection.  For example, copyins(), which
copies all in-arcs of one state to another one, blindly passed each
proposed arc to newarc(), which then spent O(N) time checking that the
second state didn't already have an identical in-arc (as it very well
might; we can't just skip duplicate detection).  So you end up with O(N^2)
work being done within the copyins() call.  moveins(), copyouts(), and
moveouts() have the same problem.

The attached re-oNsquared.patch deals with these problems by teaching
those four functions to use a sort-and-merge approach when there are
more than a few source arcs, thus taking O(N log N) time not O(N^2).
This method destroys the original ordering of the arcs, but AFAICS nothing
particularly cares about that.

That patch also adds a mergeins() function that's unused as of that patch,
but is needed for the next one; it seems to belong here because it is
pretty much like copyins() except that it copies arcs that might have come
from multiple source states.

I also fixed the brain-dead O(N^2) bubble sort algorithm used in
carcsort().  That wasn't much of a bottleneck originally, but it became so
once the other O(N^2) sore spots were gone.  (BTW, I wonder whether we
need carcsort() at all --- it doesn't look to me like the regex executor
particularly cares about the ordering of arcs either.  But I left it in
for now, so as to avoid introducing any unexpected coupling between the
compiler and executor.)

Once we'd beaten those areas down, further testing showed that
fixempties() still took an unreasonable amount of time on larger NFAs ---
in fact, it seemed to take something like O(N^4) to clean up an N-state
loop of EMPTY arcs.  I'd rewritten that once already, years ago, but
it was time to do it again.  The attached re-better-fixempties.patch
gets it down to O(N^2), which I think is the best we can do.  The win
comes partly from using mergeins() to amortize de-duplication of arcs
across multiple source states, and partly from exploiting knowledge of
the ordering of arcs for each state to avoid looking at arcs we don't
need to consider during the scan.

Also, we found out that the pullback/pushfwd stage was sometimes
creating lots and lots of excess states, because for each constraint arc
that it needed to move, it would generate a new intermediate state.
Constructs such as "\Y" generate lots of parallel constraint arcs between
the same two states, and if we need to push or pull all those arcs, we got
lots more states than we really needed.  Patch re-better-pushpull.patch
fixes that by re-using already-created intermediate states.  I also took
the trouble to redefine push() and pull() to have a less risky API: they
no longer delete any state or arc that the caller might possibly have
a pointer to, except for the specifically passed constraint arc.

Next up is re-memaccounting.patch, which replaces the previous limit
on the number of NFA states with a combined limit on the number of
states and arcs (measured by memory consumption for simplicity).
The reason we need this is that some contrived regexes involve O(N^2)
arcs despite having only O(N) states, so that it's possible to bloat the
backend to gigabytes without hitting the nfa-has-too-many-states error.
We hadn't really noticed this problem before, because the performance
problems addressed in the previous patches would ensure that you'd get
bored long before running out of memory ... but now it's possible
to get to that in a reasonable amount of time.  An example is that with
the previous patches but not this one,
        SELECT '' ~ repeat('x*y*z*', 2000);
takes about 10 minutes and 3GB.  Without the fixempties rewrite, the
runtime would probably be many days.  (BTW, do NOT try that example
without last week's cancel patches ... not only will it take forever,
but it will be uncancelable for most of that.)

With the memaccounting patch, we can fairly confidently say that regexp
compilation cannot take more than about 150MB, or half that on 32-bit
machines, no matter how crazy a pattern you throw at it.  (Note: the patch
intentionally omits counting space for the "colormap"; AFAICS that can't
grow to more than a few MB given the existing limit on number of colors,
so it doesn't seem worth the extra complexity to track its space usage
explicitly.)

Lastly, re-mopup.patch includes some documentation I wrote along the way,
some more regression test cases, and some minor fixes that didn't seem to
fit into the other categories, notably removing the no-longer-needed "all"
flags for copyins() and copyouts(), and some minor improvements in the
REG_DEBUG data structure printing logic.

I propose to push these into all supported branches, and I'll also be
sending them over to the Tcl crew who will be very happy to be able to
close out some ancient bug tracker entries.  If anyone wants to review
'em first, please do.

                        regards, tom lane

PS: patches are meant to be applied in the given order; they're not
entirely independent.

diff --git a/src/backend/regex/regc_nfa.c b/src/backend/regex/regc_nfa.c
index 50a762e..20660b4 100644
*** a/src/backend/regex/regc_nfa.c
--- b/src/backend/regex/regc_nfa.c
*************** specialcolors(struct nfa * nfa)
*** 858,863 ****
--- 858,875 ----
  
  /*
   * optimize - optimize an NFA
+  *
+  * The main goal of this function is not so much "optimization" (though it
+  * does try to get rid of useless NFA states) as reducing the NFA to a form
+  * the regex executor can handle.  The executor, and indeed the cNFA format
+  * that is its input, can only handle PLAIN and LACON arcs.  The output of
+  * the regex parser also includes EMPTY (do-nothing) arcs, as well as
+  * ^, $, AHEAD, and BEHIND constraint arcs, which we must get rid of here.
+  * We first get rid of EMPTY arcs and then deal with the constraint arcs.
+  * The hardest part of either job is to get rid of circular loops of the
+  * target arc type.  We would have to do that in any case, though, as such a
+  * loop would otherwise allow the executor to cycle through the loop endlessly
+  * without making any progress in the input string.
   */
  static long						/* re_info bits */
  optimize(struct nfa * nfa,
*************** optimize(struct nfa * nfa,
*** 881,886 ****
--- 893,899 ----
  	if (verbose)
  		fprintf(f, "\nconstraints:\n");
  #endif
+ 	fixconstraintloops(nfa, f); /* get rid of constraint loops */
  	pullback(nfa, f);			/* pull back constraints backward */
  	pushfwd(nfa, f);			/* push fwd constraints forward */
  #ifdef REG_DEBUG
*************** optimize(struct nfa * nfa,
*** 892,898 ****
  }
  
  /*
!  * pullback - pull back constraints backward to (with luck) eliminate them
   */
  static void
  pullback(struct nfa * nfa,
--- 905,911 ----
  }
  
  /*
!  * pullback - pull back constraints backward to eliminate them
   */
  static void
  pullback(struct nfa * nfa,
*************** pullback(struct nfa * nfa,
*** 926,931 ****
--- 939,950 ----
  	if (NISERR())
  		return;
  
+ 	/*
+ 	 * Any ^ constraints we were able to pull to the start state can now be
+ 	 * replaced by PLAIN arcs referencing the BOS or BOL colors.  There should
+ 	 * be no other ^ or BEHIND arcs left in the NFA, though we do not check
+ 	 * that here (compact() will fail if so).
+ 	 */
  	for (a = nfa->pre->outs; a != NULL; a = nexta)
  	{
  		nexta = a->outchain;
*************** pull(struct nfa * nfa,
*** 954,964 ****
  	struct arc *nexta;
  	struct state *s;
  
! 	if (from == to)
! 	{							/* circular constraint is pointless */
! 		freearc(nfa, con);
! 		return 1;
! 	}
  	if (from->flag)				/* can't pull back beyond start */
  		return 0;
  	if (from->nins == 0)
--- 973,979 ----
  	struct arc *nexta;
  	struct state *s;
  
! 	assert(from != to);			/* should have gotten rid of this earlier */
  	if (from->flag)				/* can't pull back beyond start */
  		return 0;
  	if (from->nins == 0)
*************** pull(struct nfa * nfa,
*** 967,999 ****
  		return 1;
  	}
  
- 	/*
- 	 * DGP 2007-11-15: Cloning a state with a circular constraint on its list
- 	 * of outs can lead to trouble [Tcl Bug 1810038], so get rid of them
- 	 * first.
- 	 */
- 	for (a = from->outs; a != NULL; a = nexta)
- 	{
- 		nexta = a->outchain;
- 		switch (a->type)
- 		{
- 			case '^':
- 			case '$':
- 			case BEHIND:
- 			case AHEAD:
- 				if (from == a->to)
- 					freearc(nfa, a);
- 				break;
- 		}
- 	}
- 
  	/* first, clone from state if necessary to avoid other outarcs */
  	if (from->nouts > 1)
  	{
  		s = newstate(nfa);
  		if (NISERR())
  			return 0;
- 		assert(to != from);		/* con is not an inarc */
  		copyins(nfa, from, s, 1);		/* duplicate inarcs */
  		cparc(nfa, con, s, to); /* move constraint arc */
  		freearc(nfa, con);
--- 982,993 ----
*************** pull(struct nfa * nfa,
*** 1036,1042 ****
  }
  
  /*
!  * pushfwd - push forward constraints forward to (with luck) eliminate them
   */
  static void
  pushfwd(struct nfa * nfa,
--- 1030,1036 ----
  }
  
  /*
!  * pushfwd - push forward constraints forward to eliminate them
   */
  static void
  pushfwd(struct nfa * nfa,
*************** pushfwd(struct nfa * nfa,
*** 1070,1075 ****
--- 1064,1075 ----
  	if (NISERR())
  		return;
  
+ 	/*
+ 	 * Any $ constraints we were able to push to the post state can now be
+ 	 * replaced by PLAIN arcs referencing the EOS or EOL colors.  There should
+ 	 * be no other $ or AHEAD arcs left in the NFA, though we do not check
+ 	 * that here (compact() will fail if so).
+ 	 */
  	for (a = nfa->post->ins; a != NULL; a = nexta)
  	{
  		nexta = a->inchain;
*************** push(struct nfa * nfa,
*** 1098,1108 ****
  	struct arc *nexta;
  	struct state *s;
  
! 	if (to == from)
! 	{							/* circular constraint is pointless */
! 		freearc(nfa, con);
! 		return 1;
! 	}
  	if (to->flag)				/* can't push forward beyond end */
  		return 0;
  	if (to->nouts == 0)
--- 1098,1104 ----
  	struct arc *nexta;
  	struct state *s;
  
! 	assert(to != from);			/* should have gotten rid of this earlier */
  	if (to->flag)				/* can't push forward beyond end */
  		return 0;
  	if (to->nouts == 0)
*************** push(struct nfa * nfa,
*** 1111,1139 ****
  		return 1;
  	}
  
- 	/*
- 	 * DGP 2007-11-15: Here we duplicate the same protections as appear in
- 	 * pull() above to avoid troubles with cloning a state with a circular
- 	 * constraint on its list of ins.  It is not clear whether this is
- 	 * necessary, or is protecting against a "can't happen". Any test case
- 	 * that actually leads to a freearc() call here would be a welcome
- 	 * addition to the test suite.
- 	 */
- 	for (a = to->ins; a != NULL; a = nexta)
- 	{
- 		nexta = a->inchain;
- 		switch (a->type)
- 		{
- 			case '^':
- 			case '$':
- 			case BEHIND:
- 			case AHEAD:
- 				if (a->from == to)
- 					freearc(nfa, a);
- 				break;
- 		}
- 	}
- 
  	/* first, clone to state if necessary to avoid other inarcs */
  	if (to->nins > 1)
  	{
--- 1107,1112 ----
*************** replaceempty(struct nfa * nfa,
*** 1458,1463 ****
--- 1431,2041 ----
  }
  
  /*
+  * isconstraintarc - detect whether an arc is of a constraint type
+  */
+ static inline int
+ isconstraintarc(struct arc * a)
+ {
+ 	switch (a->type)
+ 	{
+ 		case '^':
+ 		case '$':
+ 		case BEHIND:
+ 		case AHEAD:
+ 		case LACON:
+ 			return 1;
+ 	}
+ 	return 0;
+ }
+ 
+ /*
+  * hasconstraintout - does state have a constraint out arc?
+  */
+ static int
+ hasconstraintout(struct state * s)
+ {
+ 	struct arc *a;
+ 
+ 	for (a = s->outs; a != NULL; a = a->outchain)
+ 	{
+ 		if (isconstraintarc(a))
+ 			return 1;
+ 	}
+ 	return 0;
+ }
+ 
+ /*
+  * fixconstraintloops - get rid of loops containing only constraint arcs
+  *
+  * A loop of states that contains only constraint arcs is useless, since
+  * passing around the loop represents no forward progress.  Moreover, it
+  * would cause infinite looping in pullback/pushfwd, so we need to get rid
+  * of such loops before doing that.
+  */
+ static void
+ fixconstraintloops(struct nfa * nfa,
+ 				   FILE *f)		/* for debug output; NULL none */
+ {
+ 	struct state *s;
+ 	struct state *nexts;
+ 	struct arc *a;
+ 	struct arc *nexta;
+ 	int			hasconstraints;
+ 
+ 	/*
+ 	 * In the trivial case of a state that loops to itself, we can just drop
+ 	 * the constraint arc altogether.  This is worth special-casing because
+ 	 * such loops are far more common than loops containing multiple states.
+ 	 * While we're at it, note whether any constraint arcs survive.
+ 	 */
+ 	hasconstraints = 0;
+ 	for (s = nfa->states; s != NULL && !NISERR(); s = nexts)
+ 	{
+ 		nexts = s->next;
+ 		/* while we're at it, ensure tmp fields are clear for next step */
+ 		assert(s->tmp == NULL);
+ 		for (a = s->outs; a != NULL && !NISERR(); a = nexta)
+ 		{
+ 			nexta = a->outchain;
+ 			if (isconstraintarc(a))
+ 			{
+ 				if (a->to == s)
+ 					freearc(nfa, a);
+ 				else
+ 					hasconstraints = 1;
+ 			}
+ 		}
+ 		/* If we removed all the outarcs, the state is useless. */
+ 		if (s->nouts == 0 && !s->flag)
+ 			dropstate(nfa, s);
+ 	}
+ 
+ 	/* Nothing to do if no remaining constraint arcs */
+ 	if (NISERR() || !hasconstraints)
+ 		return;
+ 
+ 	/*
+ 	 * Starting from each remaining NFA state, search outwards for a
+ 	 * constraint loop.  If we find a loop, break the loop, then start the
+ 	 * search over.  (We could possibly retain some state from the first scan,
+ 	 * but it would complicate things greatly, and multi-state constraint
+ 	 * loops are rare enough that it's not worth optimizing the case.)
+ 	 */
+ restart:
+ 	for (s = nfa->states; s != NULL && !NISERR(); s = s->next)
+ 	{
+ 		if (findconstraintloop(nfa, s))
+ 			goto restart;
+ 	}
+ 
+ 	if (NISERR())
+ 		return;
+ 
+ 	/*
+ 	 * Now remove any states that have become useless.  (This cleanup is not
+ 	 * very thorough, and would be even less so if we tried to combine it with
+ 	 * the previous step; but cleanup() will take care of anything we miss.)
+ 	 *
+ 	 * Because findconstraintloop intentionally doesn't reset all tmp fields,
+ 	 * we have to clear them after it's done.  This is a convenient place to
+ 	 * do that, too.
+ 	 */
+ 	for (s = nfa->states; s != NULL; s = nexts)
+ 	{
+ 		nexts = s->next;
+ 		s->tmp = NULL;
+ 		if ((s->nins == 0 || s->nouts == 0) && !s->flag)
+ 			dropstate(nfa, s);
+ 	}
+ 
+ 	if (f != NULL)
+ 		dumpnfa(nfa, f);
+ }
+ 
+ /*
+  * findconstraintloop - recursively find a loop of constraint arcs
+  *
+  * If we find a loop, break it by calling breakconstraintloop(), then
+  * return 1; otherwise return 0.
+  *
+  * State tmp fields are guaranteed all NULL on a success return, because
+  * breakconstraintloop does that.  After a failure return, any state that
+  * is known not to be part of a loop is marked with s->tmp == s; this allows
+  * us not to have to re-prove that fact on later calls.  (This convention is
+  * workable because we already eliminated single-state loops.)
+  *
+  * Note that the found loop doesn't necessarily include the first state we
+  * are called on.  Any loop reachable from that state will do.
+  *
+  * The maximum recursion depth here is one more than the length of the longest
+  * loop-free chain of constraint arcs, which is surely no more than the size
+  * of the NFA, and in practice will be a lot less than that.
+  */
+ static int
+ findconstraintloop(struct nfa * nfa, struct state * s)
+ {
+ 	struct arc *a;
+ 
+ 	/* Since this is recursive, it could be driven to stack overflow */
+ 	if (STACK_TOO_DEEP(nfa->v->re))
+ 	{
+ 		NERR(REG_ETOOBIG);
+ 		return 1;				/* to exit as quickly as possible */
+ 	}
+ 
+ 	if (s->tmp != NULL)
+ 	{
+ 		/* Already proven uninteresting? */
+ 		if (s->tmp == s)
+ 			return 0;
+ 		/* Found a loop involving s */
+ 		breakconstraintloop(nfa, s);
+ 		/* The tmp fields have been cleaned up by breakconstraintloop */
+ 		return 1;
+ 	}
+ 	for (a = s->outs; a != NULL; a = a->outchain)
+ 	{
+ 		if (isconstraintarc(a))
+ 		{
+ 			struct state *sto = a->to;
+ 
+ 			assert(sto != s);
+ 			s->tmp = sto;
+ 			if (findconstraintloop(nfa, sto))
+ 				return 1;
+ 		}
+ 	}
+ 
+ 	/*
+ 	 * If we get here, no constraint loop exists leading out from s.  Mark it
+ 	 * with s->tmp == s so we need not rediscover that fact again later.
+ 	 */
+ 	s->tmp = s;
+ 	return 0;
+ }
+ 
+ /*
+  * breakconstraintloop - break a loop of constraint arcs
+  *
+  * sinitial is any one member state of the loop.  Each loop member's tmp
+  * field links to its successor within the loop.  (Note that this function
+  * will reset all the tmp fields to NULL.)
+  *
+  * We can break the loop by, for any one state S1 in the loop, cloning its
+  * loop successor state S2 (and possibly following states), and then moving
+  * all S1->S2 constraint arcs to point to the cloned S2.  The cloned S2 should
+  * copy any non-constraint outarcs of S2.  Constraint outarcs should be
+  * dropped if they point back to S1, else they need to be copied as arcs to
+  * similarly cloned states S3, S4, etc.  In general, each cloned state copies
+  * non-constraint outarcs, drops constraint outarcs that would lead to itself
+  * or any earlier cloned state, and sends other constraint outarcs to newly
+  * cloned states.  No cloned state will have any inarcs that aren't constraint
+  * arcs or do not lead from S1 or earlier-cloned states.  It's okay to drop
+  * constraint back-arcs since they would not take us to any state we've not
+  * already been in; therefore, no new constraint loop is created.  In this way
+  * we generate a modified NFA that can still represent every useful state
+  * sequence, but not sequences that represent state loops with no consumption
+  * of input data.  Note that the set of cloned states will certainly include
+  * all of the loop member states other than S1, and it may also include
+  * non-loop states that are reachable from S2 via constraint arcs.  This is
+  * important because there is no guarantee that findconstraintloop found a
+  * maximal loop (and searching for one would be NP-hard, so don't try).
+  * Frequently the "non-loop states" are actually part of a larger loop that
+  * we didn't notice, and indeed there may be several overlapping loops.
+  * This technique ensures convergence in such cases, while considering only
+  * the originally-found loop does not.
+  *
+  * If there is only one S1->S2 constraint arc, then that constraint is
+  * certainly satisfied when we enter any of the clone states.  This means that
+  * in the common case where many of the constraint arcs are identically
+  * labeled, we can merge together clone states linked by a similarly-labeled
+  * constraint: if we can get to the first one we can certainly get to the
+  * second, so there's no need to distinguish.  This greatly reduces the number
+  * of new states needed, so we preferentially break the given loop at a state
+  * pair where this is true.
+  *
+  * Furthermore, it's fairly common to find that a cloned successor state has
+  * no outarcs, especially if we're a bit aggressive about removing unnecessary
+  * outarcs.  If that happens, then there is simply not any interesting state
+  * that can be reached through the predecessor's loop arcs, which means we can
+  * break the loop just by removing those loop arcs, with no new states added.
+  */
+ static void
+ breakconstraintloop(struct nfa * nfa, struct state * sinitial)
+ {
+ 	struct state *s;
+ 	struct state *shead;
+ 	struct state *stail;
+ 	struct state *sclone;
+ 	struct state *nexts;
+ 	struct arc *refarc;
+ 	struct arc *a;
+ 	struct arc *nexta;
+ 
+ 	/*
+ 	 * Start by identifying which loop step we want to break at.
+ 	 * Preferentially this is one with only one constraint arc.  (XXX are
+ 	 * there any other secondary heuristics we want to use here?)  Set refarc
+ 	 * to point to the selected lone constraint arc, if there is one.
+ 	 */
+ 	refarc = NULL;
+ 	s = sinitial;
+ 	do
+ 	{
+ 		nexts = s->tmp;
+ 		assert(nexts != s);		/* should not see any one-element loops */
+ 		if (refarc == NULL)
+ 		{
+ 			int			narcs = 0;
+ 
+ 			for (a = s->outs; a != NULL; a = a->outchain)
+ 			{
+ 				if (a->to == nexts && isconstraintarc(a))
+ 				{
+ 					refarc = a;
+ 					narcs++;
+ 				}
+ 			}
+ 			assert(narcs > 0);
+ 			if (narcs > 1)
+ 				refarc = NULL;	/* multiple constraint arcs here, no good */
+ 		}
+ 		s = nexts;
+ 	} while (s != sinitial);
+ 
+ 	if (refarc)
+ 	{
+ 		/* break at the refarc */
+ 		shead = refarc->from;
+ 		stail = refarc->to;
+ 		assert(stail == shead->tmp);
+ 	}
+ 	else
+ 	{
+ 		/* for lack of a better idea, break after sinitial */
+ 		shead = sinitial;
+ 		stail = sinitial->tmp;
+ 	}
+ 
+ 	/*
+ 	 * Reset the tmp fields so that we can use them for local storage in
+ 	 * clonesuccessorstates.  (findconstraintloop won't mind, since it's just
+ 	 * going to abandon its search anyway.)
+ 	 */
+ 	for (s = nfa->states; s != NULL; s = s->next)
+ 		s->tmp = NULL;
+ 
+ 	/*
+ 	 * Recursively build clone state(s) as needed.
+ 	 */
+ 	sclone = newstate(nfa);
+ 	if (sclone == NULL)
+ 	{
+ 		assert(NISERR());
+ 		return;
+ 	}
+ 
+ 	clonesuccessorstates(nfa, stail, sclone, shead, refarc,
+ 						 NULL, NULL, nfa->nstates);
+ 
+ 	if (NISERR())
+ 		return;
+ 
+ 	/*
+ 	 * It's possible that sclone has no outarcs at all, in which case it's
+ 	 * useless.  (We don't try extremely hard to get rid of useless states
+ 	 * here, but this is an easy and fairly common case.)
+ 	 */
+ 	if (sclone->nouts == 0)
+ 	{
+ 		freestate(nfa, sclone);
+ 		sclone = NULL;
+ 	}
+ 
+ 	/*
+ 	 * Move shead's constraint-loop arcs to point to sclone, or just drop them
+ 	 * if we discovered we don't need sclone.
+ 	 */
+ 	for (a = shead->outs; a != NULL; a = nexta)
+ 	{
+ 		nexta = a->outchain;
+ 		if (a->to == stail && isconstraintarc(a))
+ 		{
+ 			if (sclone)
+ 				cparc(nfa, a, shead, sclone);
+ 			freearc(nfa, a);
+ 			if (NISERR())
+ 				break;
+ 		}
+ 	}
+ }
+ 
+ /*
+  * clonesuccessorstates - create a tree of constraint-arc successor states
+  *
+  * ssource is the state to be cloned, and sclone is the state to copy its
+  * outarcs into.  sclone's inarcs, if any, should already be set up.
+  *
+  * spredecessor is the original predecessor state that we are trying to build
+  * successors for (it may not be the immediate predecessor of ssource).
+  * refarc, if not NULL, is the original constraint arc that is known to have
+  * been traversed out of spredecessor to reach the successor(s).
+  *
+  * For each cloned successor state, we transiently create a "donemap" that is
+  * a boolean array showing which source states we've already visited for this
+  * clone state.  This prevents infinite recursion as well as useless repeat
+  * visits to the same state subtree (which can add up fast, since typical NFAs
+  * have multiple redundant arc pathways).  Each donemap is a char array
+  * indexed by state number.  The donemaps are all of the same size "nstates",
+  * which is nfa->nstates as of the start of the recursion.  This is enough to
+  * have entries for all pre-existing states, but *not* entries for clone
+  * states created during the recursion.  That's okay since we have no need to
+  * mark those.
+  *
+  * curdonemap is NULL when recursing to a new sclone state, or sclone's
+  * donemap when we are recursing without having created a new state (which we
+  * do when we decide we can merge a successor state into the current clone
+  * state).  outerdonemap is NULL at the top level and otherwise the parent
+  * clone state's donemap.
+  *
+  * The successor states we create and fill here form a strict tree structure,
+  * with each state having exactly one predecessor, except that the toplevel
+  * state has no inarcs as yet (breakconstraintloop will add its inarcs from
+  * spredecessor after we're done).  Thus, we can examine sclone's inarcs back
+  * to the root, plus refarc if any, to identify the set of constraints already
+  * known valid at the current point.  This allows us to avoid generating extra
+  * successor states.
+  */
+ static void
+ clonesuccessorstates(struct nfa * nfa,
+ 					 struct state * ssource,
+ 					 struct state * sclone,
+ 					 struct state * spredecessor,
+ 					 struct arc * refarc,
+ 					 char *curdonemap,
+ 					 char *outerdonemap,
+ 					 int nstates)
+ {
+ 	char	   *donemap;
+ 	struct arc *a;
+ 
+ 	/* Since this is recursive, it could be driven to stack overflow */
+ 	if (STACK_TOO_DEEP(nfa->v->re))
+ 	{
+ 		NERR(REG_ETOOBIG);
+ 		return;
+ 	}
+ 
+ 	/* If this state hasn't already got a donemap, create one */
+ 	donemap = curdonemap;
+ 	if (donemap == NULL)
+ 	{
+ 		donemap = (char *) MALLOC(nstates * sizeof(char));
+ 		if (donemap == NULL)
+ 		{
+ 			NERR(REG_ESPACE);
+ 			return;
+ 		}
+ 
+ 		if (outerdonemap != NULL)
+ 		{
+ 			/*
+ 			 * Not at outermost recursion level, so copy the outer level's
+ 			 * donemap; this ensures that we see states in process of being
+ 			 * visited at outer levels, or already merged into predecessor
+ 			 * states, as ones we shouldn't traverse back to.
+ 			 */
+ 			memcpy(donemap, outerdonemap, nstates * sizeof(char));
+ 		}
+ 		else
+ 		{
+ 			/* At outermost level, only spredecessor is off-limits */
+ 			memset(donemap, 0, nstates * sizeof(char));
+ 			assert(spredecessor->no < nstates);
+ 			donemap[spredecessor->no] = 1;
+ 		}
+ 	}
+ 
+ 	/* Mark ssource as visited in the donemap */
+ 	assert(ssource->no < nstates);
+ 	assert(donemap[ssource->no] == 0);
+ 	donemap[ssource->no] = 1;
+ 
+ 	/*
+ 	 * We proceed by first cloning all of ssource's outarcs, creating new
+ 	 * clone states as needed but not doing more with them than that.  Then in
+ 	 * a second pass, recurse to process the child clone states.  This allows
+ 	 * us to have only one child clone state per reachable source state, even
+ 	 * when there are multiple outarcs leading to the same state.  Also, when
+ 	 * we do visit a child state, its set of inarcs is known exactly, which
+ 	 * makes it safe to apply the constraint-is-already-checked optimization.
+ 	 * Also, this ensures that we've merged all the states we can into the
+ 	 * current clone before we recurse to any children, thus possibly saving
+ 	 * them from making extra images of those states.
+ 	 *
+ 	 * While this function runs, child clone states of the current state are
+ 	 * marked by setting their tmp fields to point to the original state they
+ 	 * were cloned from.  This makes it possible to detect multiple outarcs
+ 	 * leading to the same state, and also makes it easy to distinguish clone
+ 	 * states from original states (which will have tmp == NULL).
+ 	 */
+ 	for (a = ssource->outs; a != NULL && !NISERR(); a = a->outchain)
+ 	{
+ 		struct state *sto = a->to;
+ 
+ 		/*
+ 		 * We do not consider cloning successor states that have no constraint
+ 		 * outarcs; just link to them as-is.  They cannot be part of a
+ 		 * constraint loop so there is no need to make copies.  In particular,
+ 		 * this rule keeps us from trying to clone the post state, which would
+ 		 * be a bad idea.
+ 		 */
+ 		if (isconstraintarc(a) && hasconstraintout(sto))
+ 		{
+ 			struct state *prevclone;
+ 			int			canmerge;
+ 			struct arc *a2;
+ 
+ 			/*
+ 			 * Back-link constraint arcs must not be followed.  Nor is there a
+ 			 * need to revisit states previously merged into this clone.
+ 			 */
+ 			assert(sto->no < nstates);
+ 			if (donemap[sto->no] != 0)
+ 				continue;
+ 
+ 			/*
+ 			 * Check whether we already have a child clone state for this
+ 			 * source state.
+ 			 */
+ 			prevclone = NULL;
+ 			for (a2 = sclone->outs; a2 != NULL; a2 = a2->outchain)
+ 			{
+ 				if (a2->to->tmp == sto)
+ 				{
+ 					prevclone = a2->to;
+ 					break;
+ 				}
+ 			}
+ 
+ 			/*
+ 			 * If this arc is labeled the same as refarc, or the same as any
+ 			 * arc we must have traversed to get to sclone, then no additional
+ 			 * constraints need to be met to get to sto, so we should just
+ 			 * merge its outarcs into sclone.
+ 			 */
+ 			if (refarc && a->type == refarc->type && a->co == refarc->co)
+ 				canmerge = 1;
+ 			else
+ 			{
+ 				struct state *s;
+ 
+ 				canmerge = 0;
+ 				for (s = sclone; s->ins; s = s->ins->from)
+ 				{
+ 					if (s->nins == 1 &&
+ 						a->type == s->ins->type && a->co == s->ins->co)
+ 					{
+ 						canmerge = 1;
+ 						break;
+ 					}
+ 				}
+ 			}
+ 
+ 			if (canmerge)
+ 			{
+ 				/*
+ 				 * We can merge into sclone.  If we previously made a child
+ 				 * clone state, drop it; there's no need to visit it.  (This
+ 				 * can happen if ssource has multiple pathways to sto, and we
+ 				 * only just now found one that is provably a no-op.)
+ 				 */
+ 				if (prevclone)
+ 					dropstate(nfa, prevclone);	/* kills our outarc, too */
+ 
+ 				/* Recurse to merge sto's outarcs into sclone */
+ 				clonesuccessorstates(nfa,
+ 									 sto,
+ 									 sclone,
+ 									 spredecessor,
+ 									 refarc,
+ 									 donemap,
+ 									 outerdonemap,
+ 									 nstates);
+ 				/* sto should now be marked as previously visited */
+ 				assert(NISERR() || donemap[sto->no] == 1);
+ 			}
+ 			else if (prevclone)
+ 			{
+ 				/*
+ 				 * We already have a clone state for this successor, so just
+ 				 * make another arc to it.
+ 				 */
+ 				cparc(nfa, a, sclone, prevclone);
+ 			}
+ 			else
+ 			{
+ 				/*
+ 				 * We need to create a new successor clone state.
+ 				 */
+ 				struct state *stoclone;
+ 
+ 				stoclone = newstate(nfa);
+ 				if (stoclone == NULL)
+ 				{
+ 					assert(NISERR());
+ 					break;
+ 				}
+ 				/* Mark it as to what it's a clone of */
+ 				stoclone->tmp = sto;
+ 				/* ... and add the outarc leading to it */
+ 				cparc(nfa, a, sclone, stoclone);
+ 			}
+ 		}
+ 		else
+ 		{
+ 			/*
+ 			 * Non-constraint outarcs just get copied to sclone, as do outarcs
+ 			 * leading to states with no constraint outarc.
+ 			 */
+ 			cparc(nfa, a, sclone, sto);
+ 		}
+ 	}
+ 
+ 	/*
+ 	 * If we are at outer level for this clone state, recurse to all its child
+ 	 * clone states, clearing their tmp fields as we go.  (If we're not
+ 	 * outermost for sclone, leave this to be done by the outer call level.)
+ 	 * Note that if we have multiple outarcs leading to the same clone state,
+ 	 * it will only be recursed-to once.
+ 	 */
+ 	if (curdonemap == NULL)
+ 	{
+ 		for (a = sclone->outs; a != NULL && !NISERR(); a = a->outchain)
+ 		{
+ 			struct state *stoclone = a->to;
+ 			struct state *sto = stoclone->tmp;
+ 
+ 			if (sto != NULL)
+ 			{
+ 				stoclone->tmp = NULL;
+ 				clonesuccessorstates(nfa,
+ 									 sto,
+ 									 stoclone,
+ 									 spredecessor,
+ 									 refarc,
+ 									 NULL,
+ 									 donemap,
+ 									 nstates);
+ 			}
+ 		}
+ 
+ 		/* Don't forget to free sclone's donemap when done with it */
+ 		FREE(donemap);
+ 	}
+ }
+ 
+ /*
   * cleanup - clean up NFA after optimizations
   */
  static void
*************** analyze(struct nfa * nfa)
*** 1566,1572 ****
  }
  
  /*
!  * compact - compact an NFA
   */
  static void
  compact(struct nfa * nfa,
--- 2144,2150 ----
  }
  
  /*
!  * compact - construct the compact representation of an NFA
   */
  static void
  compact(struct nfa * nfa,
*************** compact(struct nfa * nfa,
*** 1636,1642 ****
  					cnfa->flags |= HASLACONS;
  					break;
  				default:
! 					assert(NOTREACHED);
  					break;
  			}
  		carcsort(first, ca - 1);
--- 2214,2220 ----
  					cnfa->flags |= HASLACONS;
  					break;
  				default:
! 					NERR(REG_ASSERT);
  					break;
  			}
  		carcsort(first, ca - 1);
diff --git a/src/backend/regex/regcomp.c b/src/backend/regex/regcomp.c
index 62f6359..bc38192 100644
*** a/src/backend/regex/regcomp.c
--- b/src/backend/regex/regcomp.c
*************** static int	combine(struct arc *, struct 
*** 155,160 ****
--- 155,168 ----
  static void fixempties(struct nfa *, FILE *);
  static struct state *emptyreachable(struct nfa *, struct state *, struct state *);
  static void replaceempty(struct nfa *, struct state *, struct state *);
+ static int	isconstraintarc(struct arc *);
+ static int	hasconstraintout(struct state *);
+ static void fixconstraintloops(struct nfa *, FILE *);
+ static int	findconstraintloop(struct nfa *, struct state *);
+ static void breakconstraintloop(struct nfa *, struct state *);
+ static void clonesuccessorstates(struct nfa *, struct state *, struct state *,
+ 					 struct state *, struct arc *,
+ 					 char *, char *, int);
  static void cleanup(struct nfa *);
  static void markreachable(struct nfa *, struct state *, struct state *, struct state *);
  static void markcanreach(struct nfa *, struct state *, struct state *, struct state *);
diff --git a/src/test/regress/expected/regex.out b/src/test/regress/expected/regex.out
index 69a2ed0..984338e 100644
*** a/src/test/regress/expected/regex.out
--- b/src/test/regress/expected/regex.out
*************** select 'a' ~ '($|^)*';
*** 160,165 ****
--- 160,215 ----
   t
  (1 row)
  
+ -- This case exposes a bug in the original fix for CVE-2007-4772
+ select 'a' ~ '(^)+^';
+  ?column? 
+ ----------
+  t
+ (1 row)
+ 
+ -- More cases of infinite loop in pullback(), not fixed by CVE-2007-4772 fix
+ select 'a' ~ '($^)+';
+  ?column? 
+ ----------
+  f
+ (1 row)
+ 
+ select 'a' ~ '(^$)*';
+  ?column? 
+ ----------
+  t
+ (1 row)
+ 
+ select 'aa bb cc' ~ '(^(?!aa))+';
+  ?column? 
+ ----------
+  f
+ (1 row)
+ 
+ select 'aa x' ~ '(^(?!aa)(?!bb)(?!cc))+';
+  ?column? 
+ ----------
+  f
+ (1 row)
+ 
+ select 'bb x' ~ '(^(?!aa)(?!bb)(?!cc))+';
+  ?column? 
+ ----------
+  f
+ (1 row)
+ 
+ select 'cc x' ~ '(^(?!aa)(?!bb)(?!cc))+';
+  ?column? 
+ ----------
+  f
+ (1 row)
+ 
+ select 'dd x' ~ '(^(?!aa)(?!bb)(?!cc))+';
+  ?column? 
+ ----------
+  t
+ (1 row)
+ 
  -- Test for infinite loop in fixempties() (Tcl bugs 3604074, 3606683)
  select 'a' ~ '((((((a)*)*)*)*)*)*';
   ?column? 
diff --git a/src/test/regress/sql/regex.sql b/src/test/regress/sql/regex.sql
index 0a07eaf..4b9c6ea 100644
*** a/src/test/regress/sql/regex.sql
--- b/src/test/regress/sql/regex.sql
*************** explain (costs off) select * from pg_pro
*** 38,43 ****
--- 38,55 ----
  -- Test for infinite loop in pullback() (CVE-2007-4772)
  select 'a' ~ '($|^)*';
  
+ -- This case exposes a bug in the original fix for CVE-2007-4772
+ select 'a' ~ '(^)+^';
+ 
+ -- More cases of infinite loop in pullback(), not fixed by CVE-2007-4772 fix
+ select 'a' ~ '($^)+';
+ select 'a' ~ '(^$)*';
+ select 'aa bb cc' ~ '(^(?!aa))+';
+ select 'aa x' ~ '(^(?!aa)(?!bb)(?!cc))+';
+ select 'bb x' ~ '(^(?!aa)(?!bb)(?!cc))+';
+ select 'cc x' ~ '(^(?!aa)(?!bb)(?!cc))+';
+ select 'dd x' ~ '(^(?!aa)(?!bb)(?!cc))+';
+ 
  -- Test for infinite loop in fixempties() (Tcl bugs 3604074, 3606683)
  select 'a' ~ '((((((a)*)*)*)*)*)*';
  select 'a' ~ '((((((a+|)+|)+|)+|)+|)+|)';
diff --git a/src/backend/regex/regc_nfa.c b/src/backend/regex/regc_nfa.c
index 20660b4..b39c699 100644
*** a/src/backend/regex/regc_nfa.c
--- b/src/backend/regex/regc_nfa.c
*************** destroystate(struct nfa * nfa,
*** 321,326 ****
--- 321,329 ----
  
  /*
   * newarc - set up a new arc within an NFA
+  *
+  * This function checks to make sure that no duplicate arcs are created.
+  * In general we never want duplicates.
   */
  static void
  newarc(struct nfa * nfa,
*************** newarc(struct nfa * nfa,
*** 344,354 ****
  		return;
  	}
  
! 	/* check for duplicates */
! 	for (a = from->outs; a != NULL; a = a->outchain)
! 		if (a->to == to && a->co == co && a->type == t)
! 			return;
  
  	a = allocarc(nfa, from);
  	if (NISERR())
  		return;
--- 347,386 ----
  		return;
  	}
  
! 	/* check for duplicate arc, using whichever chain is shorter */
! 	if (from->nouts <= to->nins)
! 	{
! 		for (a = from->outs; a != NULL; a = a->outchain)
! 			if (a->to == to && a->co == co && a->type == t)
! 				return;
! 	}
! 	else
! 	{
! 		for (a = to->ins; a != NULL; a = a->inchain)
! 			if (a->from == from && a->co == co && a->type == t)
! 				return;
! 	}
  
+ 	/* no dup, so create the arc */
+ 	createarc(nfa, t, co, from, to);
+ }
+ 
+ /*
+  * createarc - create a new arc within an NFA
+  *
+  * This function must *only* be used after verifying that there is no existing
+  * identical arc (same type/color/from/to).
+  */
+ static void
+ createarc(struct nfa * nfa,
+ 		  int t,
+ 		  pcolor co,
+ 		  struct state * from,
+ 		  struct state * to)
+ {
+ 	struct arc *a;
+ 
+ 	/* the arc is physically allocated within its from-state */
  	a = allocarc(nfa, from);
  	if (NISERR())
  		return;
*************** newarc(struct nfa * nfa,
*** 360,373 ****
  	a->from = from;
  
  	/*
! 	 * Put the new arc on the beginning, not the end, of the chains. Not only
! 	 * is this easier, it has the very useful side effect that deleting the
! 	 * most-recently-added arc is the cheapest case rather than the most
! 	 * expensive one.
  	 */
  	a->inchain = to->ins;
  	to->ins = a;
  	a->outchain = from->outs;
  	from->outs = a;
  
  	from->nouts++;
--- 392,410 ----
  	a->from = from;
  
  	/*
! 	 * Put the new arc on the beginning, not the end, of the chains; it's
! 	 * simpler here, and freearc() is the same cost either way.  See also the
! 	 * logic in moveins() and its cohorts, as well as fixempties().
  	 */
  	a->inchain = to->ins;
+ 	a->inchainRev = NULL;
+ 	if (to->ins)
+ 		to->ins->inchainRev = a;
  	to->ins = a;
  	a->outchain = from->outs;
+ 	a->outchainRev = NULL;
+ 	if (from->outs)
+ 		from->outs->outchainRev = a;
  	from->outs = a;
  
  	from->nouts++;
*************** freearc(struct nfa * nfa,
*** 433,439 ****
  {
  	struct state *from = victim->from;
  	struct state *to = victim->to;
! 	struct arc *a;
  
  	assert(victim->type != 0);
  
--- 470,476 ----
  {
  	struct state *from = victim->from;
  	struct state *to = victim->to;
! 	struct arc *predecessor;
  
  	assert(victim->type != 0);
  
*************** freearc(struct nfa * nfa,
*** 443,487 ****
  
  	/* take it off source's out-chain */
  	assert(from != NULL);
! 	assert(from->outs != NULL);
! 	a = from->outs;
! 	if (a == victim)			/* simple case:  first in chain */
  		from->outs = victim->outchain;
  	else
  	{
! 		for (; a != NULL && a->outchain != victim; a = a->outchain)
! 			continue;
! 		assert(a != NULL);
! 		a->outchain = victim->outchain;
  	}
  	from->nouts--;
  
  	/* take it off target's in-chain */
  	assert(to != NULL);
! 	assert(to->ins != NULL);
! 	a = to->ins;
! 	if (a == victim)			/* simple case:  first in chain */
  		to->ins = victim->inchain;
  	else
  	{
! 		for (; a != NULL && a->inchain != victim; a = a->inchain)
! 			continue;
! 		assert(a != NULL);
! 		a->inchain = victim->inchain;
  	}
  	to->nins--;
  
! 	/* clean up and place on free list */
  	victim->type = 0;
  	victim->from = NULL;		/* precautions... */
  	victim->to = NULL;
  	victim->inchain = NULL;
  	victim->outchain = NULL;
  	victim->freechain = from->free;
  	from->free = victim;
  }
  
  /*
   * hasnonemptyout - Does state have a non-EMPTY out arc?
   */
  static int
--- 480,583 ----
  
  	/* take it off source's out-chain */
  	assert(from != NULL);
! 	predecessor = victim->outchainRev;
! 	if (predecessor == NULL)
! 	{
! 		assert(from->outs == victim);
  		from->outs = victim->outchain;
+ 	}
  	else
  	{
! 		assert(predecessor->outchain == victim);
! 		predecessor->outchain = victim->outchain;
! 	}
! 	if (victim->outchain != NULL)
! 	{
! 		assert(victim->outchain->outchainRev == victim);
! 		victim->outchain->outchainRev = predecessor;
  	}
  	from->nouts--;
  
  	/* take it off target's in-chain */
  	assert(to != NULL);
! 	predecessor = victim->inchainRev;
! 	if (predecessor == NULL)
! 	{
! 		assert(to->ins == victim);
  		to->ins = victim->inchain;
+ 	}
  	else
  	{
! 		assert(predecessor->inchain == victim);
! 		predecessor->inchain = victim->inchain;
! 	}
! 	if (victim->inchain != NULL)
! 	{
! 		assert(victim->inchain->inchainRev == victim);
! 		victim->inchain->inchainRev = predecessor;
  	}
  	to->nins--;
  
! 	/* clean up and place on from-state's free list */
  	victim->type = 0;
  	victim->from = NULL;		/* precautions... */
  	victim->to = NULL;
  	victim->inchain = NULL;
+ 	victim->inchainRev = NULL;
  	victim->outchain = NULL;
+ 	victim->outchainRev = NULL;
  	victim->freechain = from->free;
  	from->free = victim;
  }
  
  /*
+  * changearctarget - flip an arc to have a different to state
+  *
+  * Caller must have verified that there is no pre-existing duplicate arc.
+  *
+  * Note that because we store arcs in their from state, we can't easily have
+  * a similar changearcsource function.
+  */
+ static void
+ changearctarget(struct arc * a, struct state * newto)
+ {
+ 	struct state *oldto = a->to;
+ 	struct arc *predecessor;
+ 
+ 	assert(oldto != newto);
+ 
+ 	/* take it off old target's in-chain */
+ 	assert(oldto != NULL);
+ 	predecessor = a->inchainRev;
+ 	if (predecessor == NULL)
+ 	{
+ 		assert(oldto->ins == a);
+ 		oldto->ins = a->inchain;
+ 	}
+ 	else
+ 	{
+ 		assert(predecessor->inchain == a);
+ 		predecessor->inchain = a->inchain;
+ 	}
+ 	if (a->inchain != NULL)
+ 	{
+ 		assert(a->inchain->inchainRev == a);
+ 		a->inchain->inchainRev = predecessor;
+ 	}
+ 	oldto->nins--;
+ 
+ 	a->to = newto;
+ 
+ 	/* prepend it to new target's in-chain */
+ 	a->inchain = newto->ins;
+ 	a->inchainRev = NULL;
+ 	if (newto->ins)
+ 		newto->ins->inchainRev = a;
+ 	newto->ins = a;
+ 	newto->nins++;
+ }
+ 
+ /*
   * hasnonemptyout - Does state have a non-EMPTY out arc?
   */
  static int
*************** cparc(struct nfa * nfa,
*** 561,587 ****
  }
  
  /*
   * moveins - move all in arcs of a state to another state
   *
   * You might think this could be done better by just updating the
!  * existing arcs, and you would be right if it weren't for the desire
   * for duplicate suppression, which makes it easier to just make new
   * ones to exploit the suppression built into newarc.
   */
  static void
  moveins(struct nfa * nfa,
  		struct state * oldState,
  		struct state * newState)
  {
- 	struct arc *a;
- 
  	assert(oldState != newState);
  
! 	while ((a = oldState->ins) != NULL)
  	{
! 		cparc(nfa, a, a->from, newState);
! 		freearc(nfa, a);
  	}
  	assert(oldState->nins == 0);
  	assert(oldState->ins == NULL);
  }
--- 657,903 ----
  }
  
  /*
+  * sortins - sort the in arcs of a state by from/color/type
+  */
+ static void
+ sortins(struct nfa * nfa,
+ 		struct state * s)
+ {
+ 	struct arc **sortarray;
+ 	struct arc *a;
+ 	int			n = s->nins;
+ 	int			i;
+ 
+ 	if (n <= 1)
+ 		return;					/* nothing to do */
+ 	/* make an array of arc pointers ... */
+ 	sortarray = (struct arc **) MALLOC(n * sizeof(struct arc *));
+ 	if (sortarray == NULL)
+ 	{
+ 		NERR(REG_ESPACE);
+ 		return;
+ 	}
+ 	i = 0;
+ 	for (a = s->ins; a != NULL; a = a->inchain)
+ 		sortarray[i++] = a;
+ 	assert(i == n);
+ 	/* ... sort the array */
+ 	qsort(sortarray, n, sizeof(struct arc *), sortins_cmp);
+ 	/* ... and rebuild arc list in order */
+ 	/* it seems worth special-casing first and last items to simplify loop */
+ 	a = sortarray[0];
+ 	s->ins = a;
+ 	a->inchain = sortarray[1];
+ 	a->inchainRev = NULL;
+ 	for (i = 1; i < n - 1; i++)
+ 	{
+ 		a = sortarray[i];
+ 		a->inchain = sortarray[i + 1];
+ 		a->inchainRev = sortarray[i - 1];
+ 	}
+ 	a = sortarray[i];
+ 	a->inchain = NULL;
+ 	a->inchainRev = sortarray[i - 1];
+ 	FREE(sortarray);
+ }
+ 
+ static int
+ sortins_cmp(const void *a, const void *b)
+ {
+ 	const struct arc *aa = *((const struct arc * const *) a);
+ 	const struct arc *bb = *((const struct arc * const *) b);
+ 
+ 	/* we check the fields in the order they are most likely to be different */
+ 	if (aa->from->no < bb->from->no)
+ 		return -1;
+ 	if (aa->from->no > bb->from->no)
+ 		return 1;
+ 	if (aa->co < bb->co)
+ 		return -1;
+ 	if (aa->co > bb->co)
+ 		return 1;
+ 	if (aa->type < bb->type)
+ 		return -1;
+ 	if (aa->type > bb->type)
+ 		return 1;
+ 	return 0;
+ }
+ 
+ /*
+  * sortouts - sort the out arcs of a state by to/color/type
+  */
+ static void
+ sortouts(struct nfa * nfa,
+ 		 struct state * s)
+ {
+ 	struct arc **sortarray;
+ 	struct arc *a;
+ 	int			n = s->nouts;
+ 	int			i;
+ 
+ 	if (n <= 1)
+ 		return;					/* nothing to do */
+ 	/* make an array of arc pointers ... */
+ 	sortarray = (struct arc **) MALLOC(n * sizeof(struct arc *));
+ 	if (sortarray == NULL)
+ 	{
+ 		NERR(REG_ESPACE);
+ 		return;
+ 	}
+ 	i = 0;
+ 	for (a = s->outs; a != NULL; a = a->outchain)
+ 		sortarray[i++] = a;
+ 	assert(i == n);
+ 	/* ... sort the array */
+ 	qsort(sortarray, n, sizeof(struct arc *), sortouts_cmp);
+ 	/* ... and rebuild arc list in order */
+ 	/* it seems worth special-casing first and last items to simplify loop */
+ 	a = sortarray[0];
+ 	s->outs = a;
+ 	a->outchain = sortarray[1];
+ 	a->outchainRev = NULL;
+ 	for (i = 1; i < n - 1; i++)
+ 	{
+ 		a = sortarray[i];
+ 		a->outchain = sortarray[i + 1];
+ 		a->outchainRev = sortarray[i - 1];
+ 	}
+ 	a = sortarray[i];
+ 	a->outchain = NULL;
+ 	a->outchainRev = sortarray[i - 1];
+ 	FREE(sortarray);
+ }
+ 
+ static int
+ sortouts_cmp(const void *a, const void *b)
+ {
+ 	const struct arc *aa = *((const struct arc * const *) a);
+ 	const struct arc *bb = *((const struct arc * const *) b);
+ 
+ 	/* we check the fields in the order they are most likely to be different */
+ 	if (aa->to->no < bb->to->no)
+ 		return -1;
+ 	if (aa->to->no > bb->to->no)
+ 		return 1;
+ 	if (aa->co < bb->co)
+ 		return -1;
+ 	if (aa->co > bb->co)
+ 		return 1;
+ 	if (aa->type < bb->type)
+ 		return -1;
+ 	if (aa->type > bb->type)
+ 		return 1;
+ 	return 0;
+ }
+ 
+ /*
+  * Common decision logic about whether to use arc-by-arc operations or
+  * sort/merge.  If there's just a few source arcs we cannot recoup the
+  * cost of sorting the destination arc list, no matter how large it is.
+  * Otherwise, limit the number of arc-by-arc comparisons to about 1000
+  * (a somewhat arbitrary choice, but the breakeven point would probably
+  * be machine dependent anyway).
+  */
+ #define BULK_ARC_OP_USE_SORT(nsrcarcs, ndestarcs) \
+ 	((nsrcarcs) < 4 ? 0 : ((nsrcarcs) > 32 || (ndestarcs) > 32))
+ 
+ /*
   * moveins - move all in arcs of a state to another state
   *
   * You might think this could be done better by just updating the
!  * existing arcs, and you would be right if it weren't for the need
   * for duplicate suppression, which makes it easier to just make new
   * ones to exploit the suppression built into newarc.
+  *
+  * However, if we have a whole lot of arcs to deal with, retail duplicate
+  * checks become too slow.  In that case we proceed by sorting and merging
+  * the arc lists, and then we can indeed just update the arcs in-place.
   */
  static void
  moveins(struct nfa * nfa,
  		struct state * oldState,
  		struct state * newState)
  {
  	assert(oldState != newState);
  
! 	if (!BULK_ARC_OP_USE_SORT(oldState->nins, newState->nins))
  	{
! 		/* With not too many arcs, just do them one at a time */
! 		struct arc *a;
! 
! 		while ((a = oldState->ins) != NULL)
! 		{
! 			cparc(nfa, a, a->from, newState);
! 			freearc(nfa, a);
! 		}
! 	}
! 	else
! 	{
! 		/*
! 		 * With many arcs, use a sort-merge approach.  Note changearctarget()
! 		 * will put the arc onto the front of newState's chain, so it does not
! 		 * break our walk through the sorted part of the chain.
! 		 */
! 		struct arc *oa;
! 		struct arc *na;
! 
! 		/*
! 		 * Because we bypass newarc() in this code path, we'd better include a
! 		 * cancel check.
! 		 */
! 		if (CANCEL_REQUESTED(nfa->v->re))
! 		{
! 			NERR(REG_CANCEL);
! 			return;
! 		}
! 
! 		sortins(nfa, oldState);
! 		sortins(nfa, newState);
! 		if (NISERR())
! 			return;				/* might have failed to sort */
! 		oa = oldState->ins;
! 		na = newState->ins;
! 		while (oa != NULL && na != NULL)
! 		{
! 			struct arc *a = oa;
! 
! 			switch (sortins_cmp(&oa, &na))
! 			{
! 				case -1:
! 					/* newState does not have anything matching oa */
! 					oa = oa->inchain;
! 
! 					/*
! 					 * Rather than doing createarc+freearc, we can just unlink
! 					 * and relink the existing arc struct.
! 					 */
! 					changearctarget(a, newState);
! 					break;
! 				case 0:
! 					/* match, advance in both lists */
! 					oa = oa->inchain;
! 					na = na->inchain;
! 					/* ... and drop duplicate arc from oldState */
! 					freearc(nfa, a);
! 					break;
! 				case +1:
! 					/* advance only na; oa might have a match later */
! 					na = na->inchain;
! 					break;
! 				default:
! 					assert(NOTREACHED);
! 			}
! 		}
! 		while (oa != NULL)
! 		{
! 			/* newState does not have anything matching oa */
! 			struct arc *a = oa;
! 
! 			oa = oa->inchain;
! 			changearctarget(a, newState);
! 		}
  	}
+ 
  	assert(oldState->nins == 0);
  	assert(oldState->ins == NULL);
  }
*************** copyins(struct nfa * nfa,
*** 597,610 ****
  		struct state * newState,
  		int all)
  {
- 	struct arc *a;
- 
  	assert(oldState != newState);
  
! 	for (a = oldState->ins; a != NULL; a = a->inchain)
  	{
! 		if (all || a->type != EMPTY)
! 			cparc(nfa, a, a->from, newState);
  	}
  }
  
--- 913,1099 ----
  		struct state * newState,
  		int all)
  {
  	assert(oldState != newState);
  
! 	if (!BULK_ARC_OP_USE_SORT(oldState->nins, newState->nins))
  	{
! 		/* With not too many arcs, just do them one at a time */
! 		struct arc *a;
! 
! 		for (a = oldState->ins; a != NULL; a = a->inchain)
! 			if (all || a->type != EMPTY)
! 				cparc(nfa, a, a->from, newState);
! 	}
! 	else
! 	{
! 		/*
! 		 * With many arcs, use a sort-merge approach.  Note that createarc()
! 		 * will put new arcs onto the front of newState's chain, so it does
! 		 * not break our walk through the sorted part of the chain.
! 		 */
! 		struct arc *oa;
! 		struct arc *na;
! 
! 		/*
! 		 * Because we bypass newarc() in this code path, we'd better include a
! 		 * cancel check.
! 		 */
! 		if (CANCEL_REQUESTED(nfa->v->re))
! 		{
! 			NERR(REG_CANCEL);
! 			return;
! 		}
! 
! 		sortins(nfa, oldState);
! 		sortins(nfa, newState);
! 		if (NISERR())
! 			return;				/* might have failed to sort */
! 		oa = oldState->ins;
! 		na = newState->ins;
! 		while (oa != NULL && na != NULL)
! 		{
! 			struct arc *a = oa;
! 
! 			if (!all && a->type == EMPTY)
! 			{
! 				oa = oa->inchain;
! 				continue;
! 			}
! 
! 			switch (sortins_cmp(&oa, &na))
! 			{
! 				case -1:
! 					/* newState does not have anything matching oa */
! 					oa = oa->inchain;
! 					createarc(nfa, a->type, a->co, a->from, newState);
! 					break;
! 				case 0:
! 					/* match, advance in both lists */
! 					oa = oa->inchain;
! 					na = na->inchain;
! 					break;
! 				case +1:
! 					/* advance only na; oa might have a match later */
! 					na = na->inchain;
! 					break;
! 				default:
! 					assert(NOTREACHED);
! 			}
! 		}
! 		while (oa != NULL)
! 		{
! 			/* newState does not have anything matching oa */
! 			struct arc *a = oa;
! 
! 			if (!all && a->type == EMPTY)
! 			{
! 				oa = oa->inchain;
! 				continue;
! 			}
! 
! 			oa = oa->inchain;
! 			createarc(nfa, a->type, a->co, a->from, newState);
! 		}
! 	}
! }
! 
! /*
!  * mergeins - merge a list of inarcs into a state
!  *
!  * This is much like copyins, but the source arcs are listed in an array,
!  * and are not guaranteed unique.  It's okay to clobber the array contents.
!  */
! static void
! mergeins(struct nfa * nfa,
! 		 struct state * s,
! 		 struct arc ** arcarray,
! 		 int arccount)
! {
! 	struct arc *na;
! 	int			i;
! 	int			j;
! 
! 	if (arccount <= 0)
! 		return;
! 
! 	/*
! 	 * Because we bypass newarc() in this code path, we'd better include a
! 	 * cancel check.
! 	 */
! 	if (CANCEL_REQUESTED(nfa->v->re))
! 	{
! 		NERR(REG_CANCEL);
! 		return;
! 	}
! 
! 	/* Sort existing inarcs as well as proposed new ones */
! 	sortins(nfa, s);
! 	if (NISERR())
! 		return;					/* might have failed to sort */
! 
! 	qsort(arcarray, arccount, sizeof(struct arc *), sortins_cmp);
! 
! 	/*
! 	 * arcarray very likely includes dups, so we must eliminate them.  (This
! 	 * could be folded into the next loop, but it's not worth the trouble.)
! 	 */
! 	j = 0;
! 	for (i = 1; i < arccount; i++)
! 	{
! 		switch (sortins_cmp(&arcarray[j], &arcarray[i]))
! 		{
! 			case -1:
! 				/* non-dup */
! 				arcarray[++j] = arcarray[i];
! 				break;
! 			case 0:
! 				/* dup */
! 				break;
! 			default:
! 				/* trouble */
! 				assert(NOTREACHED);
! 		}
! 	}
! 	arccount = j + 1;
! 
! 	/*
! 	 * Now merge into s' inchain.  Note that createarc() will put new arcs
! 	 * onto the front of s's chain, so it does not break our walk through the
! 	 * sorted part of the chain.
! 	 */
! 	i = 0;
! 	na = s->ins;
! 	while (i < arccount && na != NULL)
! 	{
! 		struct arc *a = arcarray[i];
! 
! 		switch (sortins_cmp(&a, &na))
! 		{
! 			case -1:
! 				/* s does not have anything matching a */
! 				createarc(nfa, a->type, a->co, a->from, s);
! 				i++;
! 				break;
! 			case 0:
! 				/* match, advance in both lists */
! 				i++;
! 				na = na->inchain;
! 				break;
! 			case +1:
! 				/* advance only na; array might have a match later */
! 				na = na->inchain;
! 				break;
! 			default:
! 				assert(NOTREACHED);
! 		}
! 	}
! 	while (i < arccount)
! 	{
! 		/* s does not have anything matching a */
! 		struct arc *a = arcarray[i];
! 
! 		createarc(nfa, a->type, a->co, a->from, s);
! 		i++;
  	}
  }
  
*************** moveouts(struct nfa * nfa,
*** 616,630 ****
  		 struct state * oldState,
  		 struct state * newState)
  {
- 	struct arc *a;
- 
  	assert(oldState != newState);
  
! 	while ((a = oldState->outs) != NULL)
  	{
! 		cparc(nfa, a, newState, a->to);
! 		freearc(nfa, a);
  	}
  }
  
  /*
--- 1105,1189 ----
  		 struct state * oldState,
  		 struct state * newState)
  {
  	assert(oldState != newState);
  
! 	if (!BULK_ARC_OP_USE_SORT(oldState->nouts, newState->nouts))
  	{
! 		/* With not too many arcs, just do them one at a time */
! 		struct arc *a;
! 
! 		while ((a = oldState->outs) != NULL)
! 		{
! 			cparc(nfa, a, newState, a->to);
! 			freearc(nfa, a);
! 		}
  	}
+ 	else
+ 	{
+ 		/*
+ 		 * With many arcs, use a sort-merge approach.  Note that createarc()
+ 		 * will put new arcs onto the front of newState's chain, so it does
+ 		 * not break our walk through the sorted part of the chain.
+ 		 */
+ 		struct arc *oa;
+ 		struct arc *na;
+ 
+ 		/*
+ 		 * Because we bypass newarc() in this code path, we'd better include a
+ 		 * cancel check.
+ 		 */
+ 		if (CANCEL_REQUESTED(nfa->v->re))
+ 		{
+ 			NERR(REG_CANCEL);
+ 			return;
+ 		}
+ 
+ 		sortouts(nfa, oldState);
+ 		sortouts(nfa, newState);
+ 		if (NISERR())
+ 			return;				/* might have failed to sort */
+ 		oa = oldState->outs;
+ 		na = newState->outs;
+ 		while (oa != NULL && na != NULL)
+ 		{
+ 			struct arc *a = oa;
+ 
+ 			switch (sortouts_cmp(&oa, &na))
+ 			{
+ 				case -1:
+ 					/* newState does not have anything matching oa */
+ 					oa = oa->outchain;
+ 					createarc(nfa, a->type, a->co, newState, a->to);
+ 					freearc(nfa, a);
+ 					break;
+ 				case 0:
+ 					/* match, advance in both lists */
+ 					oa = oa->outchain;
+ 					na = na->outchain;
+ 					/* ... and drop duplicate arc from oldState */
+ 					freearc(nfa, a);
+ 					break;
+ 				case +1:
+ 					/* advance only na; oa might have a match later */
+ 					na = na->outchain;
+ 					break;
+ 				default:
+ 					assert(NOTREACHED);
+ 			}
+ 		}
+ 		while (oa != NULL)
+ 		{
+ 			/* newState does not have anything matching oa */
+ 			struct arc *a = oa;
+ 
+ 			oa = oa->outchain;
+ 			createarc(nfa, a->type, a->co, newState, a->to);
+ 			freearc(nfa, a);
+ 		}
+ 	}
+ 
+ 	assert(oldState->nouts == 0);
+ 	assert(oldState->outs == NULL);
  }
  
  /*
*************** copyouts(struct nfa * nfa,
*** 638,651 ****
  		 struct state * newState,
  		 int all)
  {
- 	struct arc *a;
- 
  	assert(oldState != newState);
  
! 	for (a = oldState->outs; a != NULL; a = a->outchain)
  	{
! 		if (all || a->type != EMPTY)
! 			cparc(nfa, a, newState, a->to);
  	}
  }
  
--- 1197,1283 ----
  		 struct state * newState,
  		 int all)
  {
  	assert(oldState != newState);
  
! 	if (!BULK_ARC_OP_USE_SORT(oldState->nouts, newState->nouts))
  	{
! 		/* With not too many arcs, just do them one at a time */
! 		struct arc *a;
! 
! 		for (a = oldState->outs; a != NULL; a = a->outchain)
! 			if (all || a->type != EMPTY)
! 				cparc(nfa, a, newState, a->to);
! 	}
! 	else
! 	{
! 		/*
! 		 * With many arcs, use a sort-merge approach.  Note that createarc()
! 		 * will put new arcs onto the front of newState's chain, so it does
! 		 * not break our walk through the sorted part of the chain.
! 		 */
! 		struct arc *oa;
! 		struct arc *na;
! 
! 		/*
! 		 * Because we bypass newarc() in this code path, we'd better include a
! 		 * cancel check.
! 		 */
! 		if (CANCEL_REQUESTED(nfa->v->re))
! 		{
! 			NERR(REG_CANCEL);
! 			return;
! 		}
! 
! 		sortouts(nfa, oldState);
! 		sortouts(nfa, newState);
! 		if (NISERR())
! 			return;				/* might have failed to sort */
! 		oa = oldState->outs;
! 		na = newState->outs;
! 		while (oa != NULL && na != NULL)
! 		{
! 			struct arc *a = oa;
! 
! 			if (!all && a->type == EMPTY)
! 			{
! 				oa = oa->outchain;
! 				continue;
! 			}
! 
! 			switch (sortouts_cmp(&oa, &na))
! 			{
! 				case -1:
! 					/* newState does not have anything matching oa */
! 					oa = oa->outchain;
! 					createarc(nfa, a->type, a->co, newState, a->to);
! 					break;
! 				case 0:
! 					/* match, advance in both lists */
! 					oa = oa->outchain;
! 					na = na->outchain;
! 					break;
! 				case +1:
! 					/* advance only na; oa might have a match later */
! 					na = na->outchain;
! 					break;
! 				default:
! 					assert(NOTREACHED);
! 			}
! 		}
! 		while (oa != NULL)
! 		{
! 			/* newState does not have anything matching oa */
! 			struct arc *a = oa;
! 
! 			if (!all && a->type == EMPTY)
! 			{
! 				oa = oa->outchain;
! 				continue;
! 			}
! 
! 			oa = oa->outchain;
! 			createarc(nfa, a->type, a->co, newState, a->to);
! 		}
  	}
  }
  
*************** compact(struct nfa * nfa,
*** 2217,2223 ****
  					NERR(REG_ASSERT);
  					break;
  			}
! 		carcsort(first, ca - 1);
  		ca->co = COLORLESS;
  		ca->to = 0;
  		ca++;
--- 2849,2855 ----
  					NERR(REG_ASSERT);
  					break;
  			}
! 		carcsort(first, ca - first);
  		ca->co = COLORLESS;
  		ca->to = 0;
  		ca++;
*************** compact(struct nfa * nfa,
*** 2233,2263 ****
  
  /*
   * carcsort - sort compacted-NFA arcs by color
-  *
-  * Really dumb algorithm, but if the list is long enough for that to matter,
-  * you're in real trouble anyway.
   */
  static void
! carcsort(struct carc * first,
! 		 struct carc * last)
  {
! 	struct carc *p;
! 	struct carc *q;
! 	struct carc tmp;
  
! 	if (last - first <= 1)
! 		return;
  
! 	for (p = first; p <= last; p++)
! 		for (q = p; q <= last; q++)
! 			if (p->co > q->co ||
! 				(p->co == q->co && p->to > q->to))
! 			{
! 				assert(p != q);
! 				tmp = *p;
! 				*p = *q;
! 				*q = tmp;
! 			}
  }
  
  /*
--- 2865,2893 ----
  
  /*
   * carcsort - sort compacted-NFA arcs by color
   */
  static void
! carcsort(struct carc * first, size_t n)
  {
! 	if (n > 1)
! 		qsort(first, n, sizeof(struct carc), carc_cmp);
! }
  
! static int
! carc_cmp(const void *a, const void *b)
! {
! 	const struct carc *aa = (const struct carc *) a;
! 	const struct carc *bb = (const struct carc *) b;
  
! 	if (aa->co < bb->co)
! 		return -1;
! 	if (aa->co > bb->co)
! 		return +1;
! 	if (aa->to < bb->to)
! 		return -1;
! 	if (aa->to > bb->to)
! 		return +1;
! 	return 0;
  }
  
  /*
*************** dumparcs(struct state * s,
*** 2337,2370 ****
  		 FILE *f)
  {
  	int			pos;
  
! 	assert(s->nouts > 0);
! 	/* printing arcs in reverse order is usually clearer */
! 	pos = dumprarcs(s->outs, s, f, 1);
! 	if (pos != 1)
! 		fprintf(f, "\n");
! }
! 
! /*
!  * dumprarcs - dump remaining outarcs, recursively, in reverse order
!  */
! static int						/* resulting print position */
! dumprarcs(struct arc * a,
! 		  struct state * s,
! 		  FILE *f,
! 		  int pos)				/* initial print position */
! {
! 	if (a->outchain != NULL)
! 		pos = dumprarcs(a->outchain, s, f, pos);
! 	dumparc(a, s, f);
! 	if (pos == 5)
  	{
  		fprintf(f, "\n");
- 		pos = 1;
- 	}
- 	else
- 		pos++;
- 	return pos;
  }
  
  /*
--- 2967,2994 ----
  		 FILE *f)
  {
  	int			pos;
+ 	struct arc *a;
  
! 	/* printing oldest arcs first is usually clearer */
! 	a = s->outs;
! 	assert(a != NULL);
! 	while (a->outchain != NULL)
! 		a = a->outchain;
! 	pos = 1;
! 	do
  	{
+ 		dumparc(a, s, f);
+ 		if (pos == 5)
+ 		{
+ 			fprintf(f, "\n");
+ 			pos = 1;
+ 		}
+ 		else
+ 			pos++;
+ 		a = a->outchainRev;
+ 	} while (a != NULL);
+ 	if (pos != 1)
  		fprintf(f, "\n");
  }
  
  /*
diff --git a/src/backend/regex/regcomp.c b/src/backend/regex/regcomp.c
index bc38192..ad9a797 100644
*** a/src/backend/regex/regcomp.c
--- b/src/backend/regex/regcomp.c
*************** static void dropstate(struct nfa *, stru
*** 124,138 ****
--- 124,145 ----
  static void freestate(struct nfa *, struct state *);
  static void destroystate(struct nfa *, struct state *);
  static void newarc(struct nfa *, int, pcolor, struct state *, struct state *);
+ static void createarc(struct nfa *, int, pcolor, struct state *, struct state *);
  static struct arc *allocarc(struct nfa *, struct state *);
  static void freearc(struct nfa *, struct arc *);
+ static void changearctarget(struct arc *, struct state *);
  static int	hasnonemptyout(struct state *);
  static int	nonemptyouts(struct state *);
  static int	nonemptyins(struct state *);
  static struct arc *findarc(struct state *, int, pcolor);
  static void cparc(struct nfa *, struct arc *, struct state *, struct state *);
+ static void sortins(struct nfa *, struct state *);
+ static int	sortins_cmp(const void *, const void *);
+ static void sortouts(struct nfa *, struct state *);
+ static int	sortouts_cmp(const void *, const void *);
  static void moveins(struct nfa *, struct state *, struct state *);
  static void copyins(struct nfa *, struct state *, struct state *, int);
+ static void mergeins(struct nfa *, struct state *, struct arc **, int);
  static void moveouts(struct nfa *, struct state *, struct state *);
  static void copyouts(struct nfa *, struct state *, struct state *, int);
  static void cloneouts(struct nfa *, struct state *, struct state *, struct state *, int);
*************** static void markreachable(struct nfa *, 
*** 168,174 ****
  static void markcanreach(struct nfa *, struct state *, struct state *, struct state *);
  static long analyze(struct nfa *);
  static void compact(struct nfa *, struct cnfa *);
! static void carcsort(struct carc *, struct carc *);
  static void freecnfa(struct cnfa *);
  static void dumpnfa(struct nfa *, FILE *);
  
--- 175,182 ----
  static void markcanreach(struct nfa *, struct state *, struct state *, struct state *);
  static long analyze(struct nfa *);
  static void compact(struct nfa *, struct cnfa *);
! static void carcsort(struct carc *, size_t);
! static int	carc_cmp(const void *, const void *);
  static void freecnfa(struct cnfa *);
  static void dumpnfa(struct nfa *, FILE *);
  
diff --git a/src/include/regex/regguts.h b/src/include/regex/regguts.h
index 3277752..357891a 100644
*** a/src/include/regex/regguts.h
--- b/src/include/regex/regguts.h
*************** struct arc
*** 289,296 ****
  	struct state *from;			/* where it's from (and contained within) */
  	struct state *to;			/* where it's to */
  	struct arc *outchain;		/* link in *from's outs chain or free chain */
! #define  freechain	 outchain
  	struct arc *inchain;		/* link in *to's ins chain */
  	struct arc *colorchain;		/* link in color's arc chain */
  	struct arc *colorchainRev;	/* back-link in color's arc chain */
  };
--- 289,298 ----
  	struct state *from;			/* where it's from (and contained within) */
  	struct state *to;			/* where it's to */
  	struct arc *outchain;		/* link in *from's outs chain or free chain */
! 	struct arc *outchainRev;	/* back-link in *from's outs chain */
! #define  freechain	outchain	/* we do not maintain "freechainRev" */
  	struct arc *inchain;		/* link in *to's ins chain */
+ 	struct arc *inchainRev;		/* back-link in *to's ins chain */
  	struct arc *colorchain;		/* link in color's arc chain */
  	struct arc *colorchainRev;	/* back-link in color's arc chain */
  };
diff --git a/src/backend/regex/regc_nfa.c b/src/backend/regex/regc_nfa.c
index b39c699..06351bb 100644
*** a/src/backend/regex/regc_nfa.c
--- b/src/backend/regex/regc_nfa.c
*************** hasnonemptyout(struct state * s)
*** 594,633 ****
  }
  
  /*
-  * nonemptyouts - count non-EMPTY out arcs of a state
-  */
- static int
- nonemptyouts(struct state * s)
- {
- 	int			n = 0;
- 	struct arc *a;
- 
- 	for (a = s->outs; a != NULL; a = a->outchain)
- 	{
- 		if (a->type != EMPTY)
- 			n++;
- 	}
- 	return n;
- }
- 
- /*
-  * nonemptyins - count non-EMPTY in arcs of a state
-  */
- static int
- nonemptyins(struct state * s)
- {
- 	int			n = 0;
- 	struct arc *a;
- 
- 	for (a = s->ins; a != NULL; a = a->inchain)
- 	{
- 		if (a->type != EMPTY)
- 			n++;
- 	}
- 	return n;
- }
- 
- /*
   * findarc - find arc, if any, from given source with given type and color
   * If there is more than one such arc, the result is random.
   */
--- 594,599 ----
*************** fixempties(struct nfa * nfa,
*** 1856,1861 ****
--- 1822,1833 ----
  	struct state *nexts;
  	struct arc *a;
  	struct arc *nexta;
+ 	int			totalinarcs;
+ 	struct arc **inarcsorig;
+ 	struct arc **arcarray;
+ 	int			arccount;
+ 	int			prevnins;
+ 	int			nskip;
  
  	/*
  	 * First, get rid of any states whose sole out-arc is an EMPTY, since
*************** fixempties(struct nfa * nfa,
*** 1896,1936 ****
  		dropstate(nfa, s);
  	}
  
  	/*
! 	 * For each remaining NFA state, find all other states that are reachable
! 	 * from it by a chain of one or more EMPTY arcs.  Then generate new arcs
  	 * that eliminate the need for each such chain.
  	 *
! 	 * If we just do this straightforwardly, the algorithm gets slow in
! 	 * complex graphs, because the same arcs get copied to all intermediate
! 	 * states of an EMPTY chain, and then uselessly pushed repeatedly to the
! 	 * chain's final state; we waste a lot of time in newarc's duplicate
! 	 * checking.  To improve matters, we decree that any state with only EMPTY
! 	 * out-arcs is "doomed" and will not be part of the final NFA. That can be
! 	 * ensured by not adding any new out-arcs to such a state. Having ensured
! 	 * that, we need not update the state's in-arcs list either; all arcs that
! 	 * might have gotten pushed forward to it will just get pushed directly to
! 	 * successor states.  This eliminates most of the useless duplicate arcs.
  	 */
  	for (s = nfa->states; s != NULL && !NISERR(); s = s->next)
  	{
! 		for (s2 = emptyreachable(nfa, s, s); s2 != s && !NISERR(); s2 = nexts)
  		{
! 			/*
! 			 * If s2 is doomed, we decide that (1) we will always push arcs
! 			 * forward to it, not pull them back to s; and (2) we can optimize
! 			 * away the push-forward, per comment above.  So do nothing.
! 			 */
! 			if (s2->flag || hasnonemptyout(s2))
! 				replaceempty(nfa, s, s2);
  
  			/* Reset the tmp fields as we walk back */
  			nexts = s2->tmp;
  			s2->tmp = NULL;
  		}
  		s->tmp = NULL;
  	}
  
  	if (NISERR())
  		return;
  
--- 1868,1998 ----
  		dropstate(nfa, s);
  	}
  
+ 	if (NISERR())
+ 		return;
+ 
  	/*
! 	 * For each remaining NFA state, find all other states from which it is
! 	 * reachable by a chain of one or more EMPTY arcs.  Then generate new arcs
  	 * that eliminate the need for each such chain.
  	 *
! 	 * We could replace a chain of EMPTY arcs that leads from a "from" state
! 	 * to a "to" state either by pushing non-EMPTY arcs forward (linking
! 	 * directly from "from"'s predecessors to "to") or by pulling them back
! 	 * (linking directly from "from" to "to"'s successors).  We choose to
! 	 * always do the former; this choice is somewhat arbitrary, but the
! 	 * approach below requires that we uniformly do one or the other.
! 	 *
! 	 * Suppose we have a chain of N successive EMPTY arcs (where N can easily
! 	 * approach the size of the NFA).  All of the intermediate states must
! 	 * have additional inarcs and outarcs, else they'd have been removed by
! 	 * the steps above.  Assuming their inarcs are mostly not empties, we will
! 	 * add O(N^2) arcs to the NFA, since a non-EMPTY inarc leading to any one
! 	 * state in the chain must be duplicated to lead to all its successor
! 	 * states as well.  So there is no hope of doing less than O(N^2) work;
! 	 * however, we should endeavor to keep the big-O cost from being even
! 	 * worse than that, which it can easily become without care.  In
! 	 * particular, suppose we were to copy all S1's inarcs forward to S2, and
! 	 * then also to S3, and then later we consider pushing S2's inarcs forward
! 	 * to S3.  If we include the arcs already copied from S1 in that, we'd be
! 	 * doing O(N^3) work.  (The duplicate-arc elimination built into newarc()
! 	 * and its cohorts would get rid of the extra arcs, but not without cost.)
! 	 *
! 	 * We can avoid this cost by treating only arcs that existed at the start
! 	 * of this phase as candidates to be pushed forward.  To identify those,
! 	 * we remember the first inarc each state had to start with.  We rely on
! 	 * the fact that newarc() and friends put new arcs on the front of their
! 	 * to-states' inchains, and that this phase never deletes arcs, so that
! 	 * the original arcs must be the last arcs in their to-states' inchains.
! 	 *
! 	 * So the process here is that, for each state in the NFA, we gather up
! 	 * all non-EMPTY inarcs of states that can reach the target state via
! 	 * EMPTY arcs.  We then sort, de-duplicate, and merge these arcs into the
! 	 * target state's inchain.  (We can safely use sort-merge for this as long
! 	 * as we update each state's original-arcs pointer after we add arcs to
! 	 * it; the sort step of mergeins probably changed the order of the old
! 	 * arcs.)
! 	 *
! 	 * Another refinement worth making is that, because we only add non-EMPTY
! 	 * arcs during this phase, and all added arcs have the same from-state as
! 	 * the non-EMPTY arc they were cloned from, we know ahead of time that any
! 	 * states having only EMPTY outarcs will be useless for lack of outarcs
! 	 * after we drop the EMPTY arcs.  (They cannot gain non-EMPTY outarcs if
! 	 * they had none to start with.)  So we need not bother to update the
! 	 * inchains of such states at all.
  	 */
+ 
+ 	/* Remember the states' first original inarcs */
+ 	/* ... and while at it, count how many old inarcs there are altogether */
+ 	inarcsorig = (struct arc **) MALLOC(nfa->nstates * sizeof(struct arc *));
+ 	if (inarcsorig == NULL)
+ 	{
+ 		NERR(REG_ESPACE);
+ 		return;
+ 	}
+ 	totalinarcs = 0;
+ 	for (s = nfa->states; s != NULL; s = s->next)
+ 	{
+ 		inarcsorig[s->no] = s->ins;
+ 		totalinarcs += s->nins;
+ 	}
+ 
+ 	/*
+ 	 * Create a workspace for accumulating the inarcs to be added to the
+ 	 * current target state.  totalinarcs is probably a considerable
+ 	 * overestimate of the space needed, but the NFA is unlikely to be large
+ 	 * enough at this point to make it worth being smarter.
+ 	 */
+ 	arcarray = (struct arc **) MALLOC(totalinarcs * sizeof(struct arc *));
+ 	if (arcarray == NULL)
+ 	{
+ 		NERR(REG_ESPACE);
+ 		FREE(inarcsorig);
+ 		return;
+ 	}
+ 
+ 	/* And iterate over the target states */
  	for (s = nfa->states; s != NULL && !NISERR(); s = s->next)
  	{
! 		/* Ignore target states without non-EMPTY outarcs, per note above */
! 		if (!s->flag && !hasnonemptyout(s))
! 			continue;
! 
! 		/* Find predecessor states and accumulate their original inarcs */
! 		arccount = 0;
! 		for (s2 = emptyreachable(nfa, s, s, inarcsorig); s2 != s; s2 = nexts)
  		{
! 			/* Add s2's original inarcs to arcarray[], but ignore empties */
! 			for (a = inarcsorig[s2->no]; a != NULL; a = a->inchain)
! 			{
! 				if (a->type != EMPTY)
! 					arcarray[arccount++] = a;
! 			}
  
  			/* Reset the tmp fields as we walk back */
  			nexts = s2->tmp;
  			s2->tmp = NULL;
  		}
  		s->tmp = NULL;
+ 		assert(arccount <= totalinarcs);
+ 
+ 		/* Remember how many original inarcs this state has */
+ 		prevnins = s->nins;
+ 
+ 		/* Add non-duplicate inarcs to target state */
+ 		mergeins(nfa, s, arcarray, arccount);
+ 
+ 		/* Now we must update the state's inarcsorig pointer */
+ 		nskip = s->nins - prevnins;
+ 		a = s->ins;
+ 		while (nskip-- > 0)
+ 			a = a->inchain;
+ 		inarcsorig[s->no] = a;
  	}
  
+ 	FREE(arcarray);
+ 	FREE(inarcsorig);
+ 
  	if (NISERR())
  		return;
  
*************** fixempties(struct nfa * nfa,
*** 1964,1983 ****
  }
  
  /*
!  * emptyreachable - recursively find all states reachable from s by EMPTY arcs
   *
   * The return value is the last such state found.  Its tmp field links back
   * to the next-to-last such state, and so on back to s, so that all these
   * states can be located without searching the whole NFA.
   *
   * The maximum recursion depth here is equal to the length of the longest
   * loop-free chain of EMPTY arcs, which is surely no more than the size of
!  * the NFA ... but that could still be enough to cause trouble.
   */
  static struct state *
  emptyreachable(struct nfa * nfa,
  			   struct state * s,
! 			   struct state * lastfound)
  {
  	struct arc *a;
  
--- 2026,2050 ----
  }
  
  /*
!  * emptyreachable - recursively find all states that can reach s by EMPTY arcs
   *
   * The return value is the last such state found.  Its tmp field links back
   * to the next-to-last such state, and so on back to s, so that all these
   * states can be located without searching the whole NFA.
   *
+  * Since this is only used in fixempties(), we pass in the inarcsorig[] array
+  * maintained by that function.  This lets us skip over all new inarcs, which
+  * are certainly not EMPTY arcs.
+  *
   * The maximum recursion depth here is equal to the length of the longest
   * loop-free chain of EMPTY arcs, which is surely no more than the size of
!  * the NFA, and in practice will be less than that.
   */
  static struct state *
  emptyreachable(struct nfa * nfa,
  			   struct state * s,
! 			   struct state * lastfound,
! 			   struct arc ** inarcsorig)
  {
  	struct arc *a;
  
*************** emptyreachable(struct nfa * nfa,
*** 1990,2068 ****
  
  	s->tmp = lastfound;
  	lastfound = s;
! 	for (a = s->outs; a != NULL; a = a->outchain)
  	{
! 		if (a->type == EMPTY && a->to->tmp == NULL)
! 			lastfound = emptyreachable(nfa, a->to, lastfound);
  	}
  	return lastfound;
  }
  
  /*
-  * replaceempty - replace an EMPTY arc chain with some non-empty arcs
-  *
-  * The EMPTY arc(s) should be deleted later, but we can't do it here because
-  * they may still be needed to identify other arc chains during fixempties().
-  */
- static void
- replaceempty(struct nfa * nfa,
- 			 struct state * from,
- 			 struct state * to)
- {
- 	int			fromouts;
- 	int			toins;
- 
- 	assert(from != to);
- 
- 	/*
- 	 * Create replacement arcs that bypass the need for the EMPTY chain.  We
- 	 * can do this either by pushing arcs forward (linking directly from
- 	 * "from"'s predecessors to "to") or by pulling them back (linking
- 	 * directly from "from" to "to"'s successors).  In general, we choose
- 	 * whichever way creates greater fan-out or fan-in, so as to improve the
- 	 * odds of reducing the other state to zero in-arcs or out-arcs and
- 	 * thereby being able to delete it.  However, if "from" is doomed (has no
- 	 * non-EMPTY out-arcs), we must keep it so, so always push forward in that
- 	 * case.
- 	 *
- 	 * The fan-out/fan-in comparison should count only non-EMPTY arcs.  If
- 	 * "from" is doomed, we can skip counting "to"'s arcs, since we want to
- 	 * force taking the copyins path in that case.
- 	 */
- 	fromouts = nonemptyouts(from);
- 	toins = (fromouts == 0) ? 1 : nonemptyins(to);
- 
- 	if (fromouts > toins)
- 	{
- 		copyouts(nfa, to, from, 0);
- 		return;
- 	}
- 	if (fromouts < toins)
- 	{
- 		copyins(nfa, from, to, 0);
- 		return;
- 	}
- 
- 	/*
- 	 * fromouts == toins.  Decide on secondary issue: copy fewest arcs.
- 	 *
- 	 * Doesn't seem to be worth the trouble to exclude empties from these
- 	 * comparisons; that takes extra time and doesn't seem to improve the
- 	 * resulting graph much.
- 	 */
- 	if (from->nins > to->nouts)
- 	{
- 		copyouts(nfa, to, from, 0);
- 		return;
- 	}
- 	else
- 	{
- 		copyins(nfa, from, to, 0);
- 		return;
- 	}
- }
- 
- /*
   * isconstraintarc - detect whether an arc is of a constraint type
   */
  static inline int
--- 2057,2071 ----
  
  	s->tmp = lastfound;
  	lastfound = s;
! 	for (a = inarcsorig[s->no]; a != NULL; a = a->inchain)
  	{
! 		if (a->type == EMPTY && a->from->tmp == NULL)
! 			lastfound = emptyreachable(nfa, a->from, lastfound, inarcsorig);
  	}
  	return lastfound;
  }
  
  /*
   * isconstraintarc - detect whether an arc is of a constraint type
   */
  static inline int
diff --git a/src/backend/regex/regcomp.c b/src/backend/regex/regcomp.c
index ad9a797..91eb771 100644
*** a/src/backend/regex/regcomp.c
--- b/src/backend/regex/regcomp.c
*************** static struct arc *allocarc(struct nfa *
*** 129,136 ****
  static void freearc(struct nfa *, struct arc *);
  static void changearctarget(struct arc *, struct state *);
  static int	hasnonemptyout(struct state *);
- static int	nonemptyouts(struct state *);
- static int	nonemptyins(struct state *);
  static struct arc *findarc(struct state *, int, pcolor);
  static void cparc(struct nfa *, struct arc *, struct state *, struct state *);
  static void sortins(struct nfa *, struct state *);
--- 129,134 ----
*************** static int	push(struct nfa *, struct arc
*** 160,167 ****
  #define COMPATIBLE	3			/* compatible but not satisfied yet */
  static int	combine(struct arc *, struct arc *);
  static void fixempties(struct nfa *, FILE *);
! static struct state *emptyreachable(struct nfa *, struct state *, struct state *);
! static void replaceempty(struct nfa *, struct state *, struct state *);
  static int	isconstraintarc(struct arc *);
  static int	hasconstraintout(struct state *);
  static void fixconstraintloops(struct nfa *, FILE *);
--- 158,165 ----
  #define COMPATIBLE	3			/* compatible but not satisfied yet */
  static int	combine(struct arc *, struct arc *);
  static void fixempties(struct nfa *, FILE *);
! static struct state *emptyreachable(struct nfa *, struct state *,
! 			   struct state *, struct arc **);
  static int	isconstraintarc(struct arc *);
  static int	hasconstraintout(struct state *);
  static void fixconstraintloops(struct nfa *, FILE *);
diff --git a/src/backend/regex/regc_nfa.c b/src/backend/regex/regc_nfa.c
index 06351bb..5461dd3 100644
*** a/src/backend/regex/regc_nfa.c
--- b/src/backend/regex/regc_nfa.c
*************** pullback(struct nfa * nfa,
*** 1513,1518 ****
--- 1513,1519 ----
  	struct state *nexts;
  	struct arc *a;
  	struct arc *nexta;
+ 	struct state *intermediates;
  	int			progress;
  
  	/* find and pull until there are no more */
*************** pullback(struct nfa * nfa,
*** 1522,1535 ****
  		for (s = nfa->states; s != NULL && !NISERR(); s = nexts)
  		{
  			nexts = s->next;
  			for (a = s->outs; a != NULL && !NISERR(); a = nexta)
  			{
  				nexta = a->outchain;
  				if (a->type == '^' || a->type == BEHIND)
! 					if (pull(nfa, a))
  						progress = 1;
- 				assert(nexta == NULL || s->no != FREESTATE);
  			}
  		}
  		if (progress && f != NULL)
  			dumpnfa(nfa, f);
--- 1523,1547 ----
  		for (s = nfa->states; s != NULL && !NISERR(); s = nexts)
  		{
  			nexts = s->next;
+ 			intermediates = NULL;
  			for (a = s->outs; a != NULL && !NISERR(); a = nexta)
  			{
  				nexta = a->outchain;
  				if (a->type == '^' || a->type == BEHIND)
! 					if (pull(nfa, a, &intermediates))
  						progress = 1;
  			}
+ 			/* clear tmp fields of intermediate states created here */
+ 			while (intermediates != NULL)
+ 			{
+ 				struct state *ns = intermediates->tmp;
+ 
+ 				intermediates->tmp = NULL;
+ 				intermediates = ns;
+ 			}
+ 			/* if s is now useless, get rid of it */
+ 			if ((s->nins == 0 || s->nouts == 0) && !s->flag)
+ 				dropstate(nfa, s);
  		}
  		if (progress && f != NULL)
  			dumpnfa(nfa, f);
*************** pullback(struct nfa * nfa,
*** 1557,1569 ****
  
  /*
   * pull - pull a back constraint backward past its source state
!  * A significant property of this function is that it deletes at most
!  * one state -- the constraint's from state -- and only if the constraint
!  * was that state's last outarc.
   */
! static int						/* 0 couldn't, 1 could */
  pull(struct nfa * nfa,
! 	 struct arc * con)
  {
  	struct state *from = con->from;
  	struct state *to = con->to;
--- 1569,1594 ----
  
  /*
   * pull - pull a back constraint backward past its source state
!  *
!  * Returns 1 if successful (which it always is unless the source is the
!  * start state or we have an internal error), 0 if nothing happened.
!  *
!  * A significant property of this function is that it deletes no pre-existing
!  * states, and no outarcs of the constraint's from state other than the given
!  * constraint arc.  This makes the loops in pullback() safe, at the cost that
!  * we may leave useless states behind.  Therefore, we leave it to pullback()
!  * to delete such states.
!  *
!  * If the from state has multiple back-constraint outarcs, and/or multiple
!  * compatible constraint inarcs, we only need to create one new intermediate
!  * state per combination of predecessor and successor states.  *intermediates
!  * points to a list of such intermediate states for this from state (chained
!  * through their tmp fields).
   */
! static int
  pull(struct nfa * nfa,
! 	 struct arc * con,
! 	 struct state ** intermediates)
  {
  	struct state *from = con->from;
  	struct state *to = con->to;
*************** pull(struct nfa * nfa,
*** 1580,1586 ****
  		return 1;
  	}
  
! 	/* first, clone from state if necessary to avoid other outarcs */
  	if (from->nouts > 1)
  	{
  		s = newstate(nfa);
--- 1605,1615 ----
  		return 1;
  	}
  
! 	/*
! 	 * First, clone from state if necessary to avoid other outarcs.  This may
! 	 * seem wasteful, but it simplifies the logic, and we'll get rid of the
! 	 * clone state again at the bottom.
! 	 */
  	if (from->nouts > 1)
  	{
  		s = newstate(nfa);
*************** pull(struct nfa * nfa,
*** 1589,1601 ****
  		copyins(nfa, from, s, 1);		/* duplicate inarcs */
  		cparc(nfa, con, s, to); /* move constraint arc */
  		freearc(nfa, con);
  		from = s;
  		con = from->outs;
  	}
  	assert(from->nouts == 1);
  
  	/* propagate the constraint into the from state's inarcs */
! 	for (a = from->ins; a != NULL; a = nexta)
  	{
  		nexta = a->inchain;
  		switch (combine(con, a))
--- 1618,1632 ----
  		copyins(nfa, from, s, 1);		/* duplicate inarcs */
  		cparc(nfa, con, s, to); /* move constraint arc */
  		freearc(nfa, con);
+ 		if (NISERR())
+ 			return 0;
  		from = s;
  		con = from->outs;
  	}
  	assert(from->nouts == 1);
  
  	/* propagate the constraint into the from state's inarcs */
! 	for (a = from->ins; a != NULL && !NISERR(); a = nexta)
  	{
  		nexta = a->inchain;
  		switch (combine(con, a))
*************** pull(struct nfa * nfa,
*** 1606,1618 ****
  			case SATISFIED:		/* no action needed */
  				break;
  			case COMPATIBLE:	/* swap the two arcs, more or less */
! 				s = newstate(nfa);
! 				if (NISERR())
! 					return 0;
! 				cparc(nfa, a, s, to);	/* anticipate move */
  				cparc(nfa, con, a->from, s);
! 				if (NISERR())
! 					return 0;
  				freearc(nfa, a);
  				break;
  			default:
--- 1637,1659 ----
  			case SATISFIED:		/* no action needed */
  				break;
  			case COMPATIBLE:	/* swap the two arcs, more or less */
! 				/* need an intermediate state, but might have one already */
! 				for (s = *intermediates; s != NULL; s = s->tmp)
! 				{
! 					assert(s->nins > 0 && s->nouts > 0);
! 					if (s->ins->from == a->from && s->outs->to == to)
! 						break;
! 				}
! 				if (s == NULL)
! 				{
! 					s = newstate(nfa);
! 					if (NISERR())
! 						return 0;
! 					s->tmp = *intermediates;
! 					*intermediates = s;
! 				}
  				cparc(nfa, con, a->from, s);
! 				cparc(nfa, a, s, to);
  				freearc(nfa, a);
  				break;
  			default:
*************** pull(struct nfa * nfa,
*** 1623,1629 ****
  
  	/* remaining inarcs, if any, incorporate the constraint */
  	moveins(nfa, from, to);
! 	dropstate(nfa, from);		/* will free the constraint */
  	return 1;
  }
  
--- 1664,1671 ----
  
  	/* remaining inarcs, if any, incorporate the constraint */
  	moveins(nfa, from, to);
! 	freearc(nfa, con);
! 	/* from state is now useless, but we leave it to pullback() to clean up */
  	return 1;
  }
  
*************** pushfwd(struct nfa * nfa,
*** 1638,1643 ****
--- 1680,1686 ----
  	struct state *nexts;
  	struct arc *a;
  	struct arc *nexta;
+ 	struct state *intermediates;
  	int			progress;
  
  	/* find and push until there are no more */
*************** pushfwd(struct nfa * nfa,
*** 1647,1660 ****
  		for (s = nfa->states; s != NULL && !NISERR(); s = nexts)
  		{
  			nexts = s->next;
  			for (a = s->ins; a != NULL && !NISERR(); a = nexta)
  			{
  				nexta = a->inchain;
  				if (a->type == '$' || a->type == AHEAD)
! 					if (push(nfa, a))
  						progress = 1;
- 				assert(nexta == NULL || s->no != FREESTATE);
  			}
  		}
  		if (progress && f != NULL)
  			dumpnfa(nfa, f);
--- 1690,1714 ----
  		for (s = nfa->states; s != NULL && !NISERR(); s = nexts)
  		{
  			nexts = s->next;
+ 			intermediates = NULL;
  			for (a = s->ins; a != NULL && !NISERR(); a = nexta)
  			{
  				nexta = a->inchain;
  				if (a->type == '$' || a->type == AHEAD)
! 					if (push(nfa, a, &intermediates))
  						progress = 1;
  			}
+ 			/* clear tmp fields of intermediate states created here */
+ 			while (intermediates != NULL)
+ 			{
+ 				struct state *ns = intermediates->tmp;
+ 
+ 				intermediates->tmp = NULL;
+ 				intermediates = ns;
+ 			}
+ 			/* if s is now useless, get rid of it */
+ 			if ((s->nins == 0 || s->nouts == 0) && !s->flag)
+ 				dropstate(nfa, s);
  		}
  		if (progress && f != NULL)
  			dumpnfa(nfa, f);
*************** pushfwd(struct nfa * nfa,
*** 1682,1694 ****
  
  /*
   * push - push a forward constraint forward past its destination state
!  * A significant property of this function is that it deletes at most
!  * one state -- the constraint's to state -- and only if the constraint
!  * was that state's last inarc.
   */
! static int						/* 0 couldn't, 1 could */
  push(struct nfa * nfa,
! 	 struct arc * con)
  {
  	struct state *from = con->from;
  	struct state *to = con->to;
--- 1736,1761 ----
  
  /*
   * push - push a forward constraint forward past its destination state
!  *
!  * Returns 1 if successful (which it always is unless the destination is the
!  * post state or we have an internal error), 0 if nothing happened.
!  *
!  * A significant property of this function is that it deletes no pre-existing
!  * states, and no inarcs of the constraint's to state other than the given
!  * constraint arc.  This makes the loops in pushfwd() safe, at the cost that
!  * we may leave useless states behind.  Therefore, we leave it to pushfwd()
!  * to delete such states.
!  *
!  * If the to state has multiple forward-constraint inarcs, and/or multiple
!  * compatible constraint outarcs, we only need to create one new intermediate
!  * state per combination of predecessor and successor states.  *intermediates
!  * points to a list of such intermediate states for this to state (chained
!  * through their tmp fields).
   */
! static int
  push(struct nfa * nfa,
! 	 struct arc * con,
! 	 struct state ** intermediates)
  {
  	struct state *from = con->from;
  	struct state *to = con->to;
*************** push(struct nfa * nfa,
*** 1705,1726 ****
  		return 1;
  	}
  
! 	/* first, clone to state if necessary to avoid other inarcs */
  	if (to->nins > 1)
  	{
  		s = newstate(nfa);
  		if (NISERR())
  			return 0;
  		copyouts(nfa, to, s, 1);	/* duplicate outarcs */
! 		cparc(nfa, con, from, s);		/* move constraint */
  		freearc(nfa, con);
  		to = s;
  		con = to->ins;
  	}
  	assert(to->nins == 1);
  
  	/* propagate the constraint into the to state's outarcs */
! 	for (a = to->outs; a != NULL; a = nexta)
  	{
  		nexta = a->outchain;
  		switch (combine(con, a))
--- 1772,1799 ----
  		return 1;
  	}
  
! 	/*
! 	 * First, clone to state if necessary to avoid other inarcs.  This may
! 	 * seem wasteful, but it simplifies the logic, and we'll get rid of the
! 	 * clone state again at the bottom.
! 	 */
  	if (to->nins > 1)
  	{
  		s = newstate(nfa);
  		if (NISERR())
  			return 0;
  		copyouts(nfa, to, s, 1);	/* duplicate outarcs */
! 		cparc(nfa, con, from, s);		/* move constraint arc */
  		freearc(nfa, con);
+ 		if (NISERR())
+ 			return 0;
  		to = s;
  		con = to->ins;
  	}
  	assert(to->nins == 1);
  
  	/* propagate the constraint into the to state's outarcs */
! 	for (a = to->outs; a != NULL && !NISERR(); a = nexta)
  	{
  		nexta = a->outchain;
  		switch (combine(con, a))
*************** push(struct nfa * nfa,
*** 1731,1743 ****
  			case SATISFIED:		/* no action needed */
  				break;
  			case COMPATIBLE:	/* swap the two arcs, more or less */
! 				s = newstate(nfa);
! 				if (NISERR())
! 					return 0;
! 				cparc(nfa, con, s, a->to);		/* anticipate move */
  				cparc(nfa, a, from, s);
- 				if (NISERR())
- 					return 0;
  				freearc(nfa, a);
  				break;
  			default:
--- 1804,1826 ----
  			case SATISFIED:		/* no action needed */
  				break;
  			case COMPATIBLE:	/* swap the two arcs, more or less */
! 				/* need an intermediate state, but might have one already */
! 				for (s = *intermediates; s != NULL; s = s->tmp)
! 				{
! 					assert(s->nins > 0 && s->nouts > 0);
! 					if (s->ins->from == from && s->outs->to == a->to)
! 						break;
! 				}
! 				if (s == NULL)
! 				{
! 					s = newstate(nfa);
! 					if (NISERR())
! 						return 0;
! 					s->tmp = *intermediates;
! 					*intermediates = s;
! 				}
! 				cparc(nfa, con, s, a->to);
  				cparc(nfa, a, from, s);
  				freearc(nfa, a);
  				break;
  			default:
*************** push(struct nfa * nfa,
*** 1748,1754 ****
  
  	/* remaining outarcs, if any, incorporate the constraint */
  	moveouts(nfa, to, from);
! 	dropstate(nfa, to);			/* will free the constraint */
  	return 1;
  }
  
--- 1831,1838 ----
  
  	/* remaining outarcs, if any, incorporate the constraint */
  	moveouts(nfa, to, from);
! 	freearc(nfa, con);
! 	/* to state is now useless, but we leave it to pushfwd() to clean up */
  	return 1;
  }
  
diff --git a/src/backend/regex/regcomp.c b/src/backend/regex/regcomp.c
index 91eb771..6b03107 100644
*** a/src/backend/regex/regcomp.c
--- b/src/backend/regex/regcomp.c
*************** static void cleartraverse(struct nfa *, 
*** 149,157 ****
  static void specialcolors(struct nfa *);
  static long optimize(struct nfa *, FILE *);
  static void pullback(struct nfa *, FILE *);
! static int	pull(struct nfa *, struct arc *);
  static void pushfwd(struct nfa *, FILE *);
! static int	push(struct nfa *, struct arc *);
  
  #define INCOMPATIBLE	1		/* destroys arc */
  #define SATISFIED	2			/* constraint satisfied */
--- 149,157 ----
  static void specialcolors(struct nfa *);
  static long optimize(struct nfa *, FILE *);
  static void pullback(struct nfa *, FILE *);
! static int	pull(struct nfa *, struct arc *, struct state **);
  static void pushfwd(struct nfa *, FILE *);
! static int	push(struct nfa *, struct arc *, struct state **);
  
  #define INCOMPATIBLE	1		/* destroys arc */
  #define SATISFIED	2			/* constraint satisfied */
diff --git a/src/backend/regex/regc_nfa.c b/src/backend/regex/regc_nfa.c
index 5461dd3..bdd00fa 100644
*** a/src/backend/regex/regc_nfa.c
--- b/src/backend/regex/regc_nfa.c
*************** newnfa(struct vars * v,
*** 63,69 ****
  	nfa->nstates = 0;
  	nfa->cm = cm;
  	nfa->v = v;
- 	nfa->size = 0;
  	nfa->bos[0] = nfa->bos[1] = COLORLESS;
  	nfa->eos[0] = nfa->eos[1] = COLORLESS;
  	nfa->parent = parent;		/* Precedes newfstate so parent is valid. */
--- 63,68 ----
*************** newnfa(struct vars * v,
*** 93,149 ****
  }
  
  /*
-  * TooManyStates - checks if the max states exceeds the compile-time value
-  */
- static int
- TooManyStates(struct nfa * nfa)
- {
- 	struct nfa *parent = nfa->parent;
- 	size_t		sz = nfa->size;
- 
- 	while (parent != NULL)
- 	{
- 		sz = parent->size;
- 		parent = parent->parent;
- 	}
- 	if (sz > REG_MAX_STATES)
- 		return 1;
- 	return 0;
- }
- 
- /*
-  * IncrementSize - increases the tracked size of the NFA and its parents.
-  */
- static void
- IncrementSize(struct nfa * nfa)
- {
- 	struct nfa *parent = nfa->parent;
- 
- 	nfa->size++;
- 	while (parent != NULL)
- 	{
- 		parent->size++;
- 		parent = parent->parent;
- 	}
- }
- 
- /*
-  * DecrementSize - decreases the tracked size of the NFA and its parents.
-  */
- static void
- DecrementSize(struct nfa * nfa)
- {
- 	struct nfa *parent = nfa->parent;
- 
- 	nfa->size--;
- 	while (parent != NULL)
- 	{
- 		parent->size--;
- 		parent = parent->parent;
- 	}
- }
- 
- /*
   * freenfa - free an entire NFA
   */
  static void
--- 92,97 ----
*************** newstate(struct nfa * nfa)
*** 188,199 ****
  		return NULL;
  	}
  
- 	if (TooManyStates(nfa))
- 	{
- 		NERR(REG_ETOOBIG);
- 		return NULL;
- 	}
- 
  	if (nfa->free != NULL)
  	{
  		s = nfa->free;
--- 136,141 ----
*************** newstate(struct nfa * nfa)
*** 201,212 ****
--- 143,160 ----
  	}
  	else
  	{
+ 		if (nfa->v->spaceused >= REG_MAX_COMPILE_SPACE)
+ 		{
+ 			NERR(REG_ETOOBIG);
+ 			return NULL;
+ 		}
  		s = (struct state *) MALLOC(sizeof(struct state));
  		if (s == NULL)
  		{
  			NERR(REG_ESPACE);
  			return NULL;
  		}
+ 		nfa->v->spaceused += sizeof(struct state);
  		s->oas.next = NULL;
  		s->free = NULL;
  		s->noas = 0;
*************** newstate(struct nfa * nfa)
*** 230,237 ****
  	}
  	s->prev = nfa->slast;
  	nfa->slast = s;
- 	/* track the current size and the parent size */
- 	IncrementSize(nfa);
  	return s;
  }
  
--- 178,183 ----
*************** freestate(struct nfa * nfa,
*** 294,300 ****
  	s->prev = NULL;
  	s->next = nfa->free;		/* don't delete it, put it on the free list */
  	nfa->free = s;
- 	DecrementSize(nfa);
  }
  
  /*
--- 240,245 ----
*************** destroystate(struct nfa * nfa,
*** 312,322 ****
--- 257,269 ----
  	{
  		abnext = ab->next;
  		FREE(ab);
+ 		nfa->v->spaceused -= sizeof(struct arcbatch);
  	}
  	s->ins = NULL;
  	s->outs = NULL;
  	s->next = NULL;
  	FREE(s);
+ 	nfa->v->spaceused -= sizeof(struct state);
  }
  
  /*
*************** allocarc(struct nfa * nfa,
*** 437,448 ****
--- 384,401 ----
  		struct arcbatch *newAb;
  		int			i;
  
+ 		if (nfa->v->spaceused >= REG_MAX_COMPILE_SPACE)
+ 		{
+ 			NERR(REG_ETOOBIG);
+ 			return NULL;
+ 		}
  		newAb = (struct arcbatch *) MALLOC(sizeof(struct arcbatch));
  		if (newAb == NULL)
  		{
  			NERR(REG_ESPACE);
  			return NULL;
  		}
+ 		nfa->v->spaceused += sizeof(struct arcbatch);
  		newAb->next = s->oas.next;
  		s->oas.next = newAb;
  
diff --git a/src/backend/regex/regcomp.c b/src/backend/regex/regcomp.c
index 6b03107..324fea5 100644
*** a/src/backend/regex/regcomp.c
--- b/src/backend/regex/regcomp.c
*************** struct vars
*** 248,253 ****
--- 248,254 ----
  	struct cvec *cv2;			/* utility cvec */
  	struct subre *lacons;		/* lookahead-constraint vector */
  	int			nlacons;		/* size of lacons */
+ 	size_t		spaceused;		/* approx. space used for compilation */
  };
  
  /* parsing macros; most know that `v' is the struct vars pointer */
*************** pg_regcomp(regex_t *re,
*** 363,368 ****
--- 364,370 ----
  	v->cv2 = NULL;
  	v->lacons = NULL;
  	v->nlacons = 0;
+ 	v->spaceused = 0;
  	re->re_magic = REMAGIC;
  	re->re_info = 0;			/* bits get set during parse */
  	re->re_csize = sizeof(chr);
diff --git a/src/include/regex/regerrs.h b/src/include/regex/regerrs.h
index 809b511..41e25f7 100644
*** a/src/include/regex/regerrs.h
--- b/src/include/regex/regerrs.h
***************
*** 75,81 ****
  },
  
  {
! 	REG_ETOOBIG, "REG_ETOOBIG", "nfa has too many states"
  },
  
  {
--- 75,81 ----
  },
  
  {
! 	REG_ETOOBIG, "REG_ETOOBIG", "regular expression is too complex"
  },
  
  {
diff --git a/src/include/regex/regex.h b/src/include/regex/regex.h
index 3020b0f..5e1b692 100644
*** a/src/include/regex/regex.h
--- b/src/include/regex/regex.h
*************** typedef struct
*** 152,158 ****
  #define REG_INVARG	16			/* invalid argument to regex function */
  #define REG_MIXED	17			/* character widths of regex and string differ */
  #define REG_BADOPT	18			/* invalid embedded option */
! #define REG_ETOOBIG 19			/* nfa has too many states */
  #define REG_ECOLORS 20			/* too many colors */
  #define REG_CANCEL	21			/* operation cancelled */
  /* two specials for debugging and testing */
--- 152,158 ----
  #define REG_INVARG	16			/* invalid argument to regex function */
  #define REG_MIXED	17			/* character widths of regex and string differ */
  #define REG_BADOPT	18			/* invalid embedded option */
! #define REG_ETOOBIG 19			/* regular expression is too complex */
  #define REG_ECOLORS 20			/* too many colors */
  #define REG_CANCEL	21			/* operation cancelled */
  /* two specials for debugging and testing */
diff --git a/src/include/regex/regguts.h b/src/include/regex/regguts.h
index 357891a..19fe991 100644
*** a/src/include/regex/regguts.h
--- b/src/include/regex/regguts.h
*************** struct nfa
*** 334,342 ****
  	struct colormap *cm;		/* the color map */
  	color		bos[2];			/* colors, if any, assigned to BOS and BOL */
  	color		eos[2];			/* colors, if any, assigned to EOS and EOL */
- 	size_t		size;			/* Current NFA size; differs from nstates as
- 								 * it also counts the number of states in
- 								 * children of this NFA. */
  	struct vars *v;				/* simplifies compile error reporting */
  	struct nfa *parent;			/* parent NFA, if any */
  };
--- 334,339 ----
*************** struct cnfa
*** 384,393 ****
  #define NULLCNFA(cnfa)	((cnfa).nstates == 0)
  
  /*
!  * Used to limit the maximum NFA size to something sane. [Tcl Bug 1810264]
   */
! #ifndef REG_MAX_STATES
! #define REG_MAX_STATES	100000
  #endif
  
  /*
--- 381,396 ----
  #define NULLCNFA(cnfa)	((cnfa).nstates == 0)
  
  /*
!  * This symbol limits the transient heap space used by the regex compiler,
!  * and thereby also the maximum complexity of NFAs that we'll deal with.
!  * Currently we only count NFA states and arcs against this; the other
!  * transient data is generally not large enough to notice compared to those.
!  * Note that we do not charge anything for the final output data structures
!  * (the compacted NFA and the colormap).
   */
! #ifndef REG_MAX_COMPILE_SPACE
! #define REG_MAX_COMPILE_SPACE  \
! 	(100000 * sizeof(struct state) + 100000 * sizeof(struct arcbatch))
  #endif
  
  /*
diff --git a/src/backend/regex/README b/src/backend/regex/README
index 29521c6..5c24d3d 100644
*** a/src/backend/regex/README
--- b/src/backend/regex/README
*************** relates to what you'll see in the code. 
*** 76,86 ****
  of states approximately proportional to the length of the regexp.
  
  * The NFA is then optimized into a "compact NFA" representation, which is
! basically the same data but without fields that are not going to be needed
! at runtime.  We do a little bit of cleanup too, such as removing
! unreachable states that might be created as a result of the rather naive
! transformation done by initial parsing.  The cNFA representation is what
! is passed from regcomp to regexec.
  
  * Unlike traditional NFA-based regex engines, we do not execute directly
  from the NFA representation, as that would require backtracking and so be
--- 76,85 ----
  of states approximately proportional to the length of the regexp.
  
  * The NFA is then optimized into a "compact NFA" representation, which is
! basically the same idea but without fields that are not going to be needed
! at runtime.  It is simplified too: the compact format only allows "plain"
! and "LACON" arc types.  The cNFA representation is what is passed from
! regcomp to regexec.
  
  * Unlike traditional NFA-based regex engines, we do not execute directly
  from the NFA representation, as that would require backtracking and so be
*************** a possible division of the input string 
*** 139,150 ****
  each match their part of the string (and although this specific case can
  only succeed when the division is at the middle, the code does not know
  that, nor would it be true in general).  However, we can first run the DFA
! and quickly reject any input that doesn't contain two a's and some number
! of b's and c's.  If the DFA doesn't match, there is no need to recurse to
! the two child nodes for each possible string division point.  In many
! cases, this prefiltering makes the search run much faster than a pure NFA
! engine could do.  It is this behavior that justifies using the phrase
! "hybrid DFA/NFA engine" to describe Spencer's library.
  
  
  Colors and colormapping
--- 138,150 ----
  each match their part of the string (and although this specific case can
  only succeed when the division is at the middle, the code does not know
  that, nor would it be true in general).  However, we can first run the DFA
! and quickly reject any input that doesn't start with an "a" and contain
! one more "a" plus some number of b's and c's.  If the DFA doesn't match,
! there is no need to recurse to the two child nodes for each possible
! string division point.  In many cases, this prefiltering makes the search
! run much faster than a pure NFA engine could do.  It is this behavior that
! justifies using the phrase "hybrid DFA/NFA engine" to describe Spencer's
! library.
  
  
  Colors and colormapping
*************** character classes are somehow processed 
*** 296,298 ****
--- 296,371 ----
  full expansion of their contents at parse time.  This would mean that we'd
  have to be ready to call iswalpha() at runtime, but if that only happens
  for high-code-value characters, it shouldn't be a big performance hit.
+ 
+ 
+ Detailed semantics of an NFA
+ ----------------------------
+ 
+ When trying to read dumped-out NFAs, it's helpful to know these facts:
+ 
+ State 0 (additionally marked with "@" in dumpnfa's output) is always the
+ goal state, and state 1 (additionally marked with ">") is the start state.
+ (The code refers to these as the post state and pre state respectively.)
+ 
+ The possible arc types are:
+ 
+     PLAIN arcs, which specify matching of any character of a given "color"
+     (see above).  These are dumped as "[color_number]->to_state".
+ 
+     EMPTY arcs, which specify a no-op transition to another state.  These
+     are dumped as "->to_state".
+ 
+     AHEAD constraints, which represent a "next character must be of this
+     color" constraint.  AHEAD differs from a PLAIN arc in that the input
+     character is not consumed when crossing the arc.  These are dumped as
+     ">color_number>->to_state".
+ 
+     BEHIND constraints, which represent a "previous character must be of
+     this color" constraint, which likewise consumes no input.  These are
+     dumped as "<color_number<->to_state".
+ 
+     '^' arcs, which specify a beginning-of-input constraint.  These are
+     dumped as "^0->to_state" or "^1->to_state" for beginning-of-string and
+     beginning-of-line constraints respectively.
+ 
+     '$' arcs, which specify an end-of-input constraint.  These are dumped
+     as "$0->to_state" or "$1->to_state" for end-of-string and end-of-line
+     constraints respectively.
+ 
+     LACON constraints, which represent "(?=re)" and "(?!re)" constraints,
+     i.e. the input starting at this point must match (or not match) a
+     given sub-RE, but the matching input is not consumed.  These are
+     dumped as ":subtree_number:->to_state".
+ 
+ If you see anything else (especially any question marks) in the display of
+ an arc, it's dumpnfa() trying to tell you that there's something fishy
+ about the arc; see the source code.
+ 
+ The regex executor can only handle PLAIN and LACON transitions.  The regex
+ optimize() function is responsible for transforming the parser's output
+ to get rid of all the other arc types.  In particular, ^ and $ arcs that
+ are not dropped as impossible will always end up adjacent to the pre or
+ post state respectively, and then will be converted into PLAIN arcs that
+ mention the special "colors" for BOS, BOL, EOS, or EOL.
+ 
+ To decide whether a thus-transformed NFA matches a given substring of the
+ input string, the executor essentially follows these rules:
+ 1. Start the NFA "looking at" the character *before* the given substring,
+ or if the substring is at the start of the input, prepend an imaginary BOS
+ character instead.
+ 2. Run the NFA until it has consumed the character *after* the given
+ substring, or an imaginary following EOS character if the substring is at
+ the end of the input.
+ 3. If the NFA is (or can be) in the goal state at this point, it matches.
+ 
+ So one can mentally execute an untransformed NFA by taking ^ and $ as
+ ordinary constraints that match at start and end of input; but plain
+ arcs out of the start state should be taken as matches for the character
+ before the target substring, and similarly, plain arcs leading to the
+ post state are matches for the character after the target substring.
+ This definition is necessary to support regexes that begin or end with
+ constraints such as \m and \M, which imply requirements on the adjacent
+ character if any.  NFAs for simple unanchored patterns will usually have
+ pre-state outarcs for all possible character colors as well as BOS and
+ BOL, and post-state inarcs for all possible character colors as well as
+ EOS and EOL, so that the executor's behavior will work.
diff --git a/src/backend/regex/regc_nfa.c b/src/backend/regex/regc_nfa.c
index bdd00fa..e3cd9e6 100644
*** a/src/backend/regex/regc_nfa.c
--- b/src/backend/regex/regc_nfa.c
*************** moveins(struct nfa * nfa,
*** 823,836 ****
  
  /*
   * copyins - copy in arcs of a state to another state
-  *
-  * Either all arcs, or only non-empty ones as determined by all value.
   */
  static void
  copyins(struct nfa * nfa,
  		struct state * oldState,
! 		struct state * newState,
! 		int all)
  {
  	assert(oldState != newState);
  
--- 823,833 ----
  
  /*
   * copyins - copy in arcs of a state to another state
   */
  static void
  copyins(struct nfa * nfa,
  		struct state * oldState,
! 		struct state * newState)
  {
  	assert(oldState != newState);
  
*************** copyins(struct nfa * nfa,
*** 840,847 ****
  		struct arc *a;
  
  		for (a = oldState->ins; a != NULL; a = a->inchain)
! 			if (all || a->type != EMPTY)
! 				cparc(nfa, a, a->from, newState);
  	}
  	else
  	{
--- 837,843 ----
  		struct arc *a;
  
  		for (a = oldState->ins; a != NULL; a = a->inchain)
! 			cparc(nfa, a, a->from, newState);
  	}
  	else
  	{
*************** copyins(struct nfa * nfa,
*** 873,884 ****
  		{
  			struct arc *a = oa;
  
- 			if (!all && a->type == EMPTY)
- 			{
- 				oa = oa->inchain;
- 				continue;
- 			}
- 
  			switch (sortins_cmp(&oa, &na))
  			{
  				case -1:
--- 869,874 ----
*************** copyins(struct nfa * nfa,
*** 904,915 ****
  			/* newState does not have anything matching oa */
  			struct arc *a = oa;
  
- 			if (!all && a->type == EMPTY)
- 			{
- 				oa = oa->inchain;
- 				continue;
- 			}
- 
  			oa = oa->inchain;
  			createarc(nfa, a->type, a->co, a->from, newState);
  		}
--- 894,899 ----
*************** moveouts(struct nfa * nfa,
*** 1107,1120 ****
  
  /*
   * copyouts - copy out arcs of a state to another state
-  *
-  * Either all arcs, or only non-empty ones as determined by all value.
   */
  static void
  copyouts(struct nfa * nfa,
  		 struct state * oldState,
! 		 struct state * newState,
! 		 int all)
  {
  	assert(oldState != newState);
  
--- 1091,1101 ----
  
  /*
   * copyouts - copy out arcs of a state to another state
   */
  static void
  copyouts(struct nfa * nfa,
  		 struct state * oldState,
! 		 struct state * newState)
  {
  	assert(oldState != newState);
  
*************** copyouts(struct nfa * nfa,
*** 1124,1131 ****
  		struct arc *a;
  
  		for (a = oldState->outs; a != NULL; a = a->outchain)
! 			if (all || a->type != EMPTY)
! 				cparc(nfa, a, newState, a->to);
  	}
  	else
  	{
--- 1105,1111 ----
  		struct arc *a;
  
  		for (a = oldState->outs; a != NULL; a = a->outchain)
! 			cparc(nfa, a, newState, a->to);
  	}
  	else
  	{
*************** copyouts(struct nfa * nfa,
*** 1157,1168 ****
  		{
  			struct arc *a = oa;
  
- 			if (!all && a->type == EMPTY)
- 			{
- 				oa = oa->outchain;
- 				continue;
- 			}
- 
  			switch (sortouts_cmp(&oa, &na))
  			{
  				case -1:
--- 1137,1142 ----
*************** copyouts(struct nfa * nfa,
*** 1188,1199 ****
  			/* newState does not have anything matching oa */
  			struct arc *a = oa;
  
- 			if (!all && a->type == EMPTY)
- 			{
- 				oa = oa->outchain;
- 				continue;
- 			}
- 
  			oa = oa->outchain;
  			createarc(nfa, a->type, a->co, newState, a->to);
  		}
--- 1162,1167 ----
*************** optimize(struct nfa * nfa,
*** 1452,1457 ****
--- 1420,1429 ----
  		fprintf(f, "\nfinal cleanup:\n");
  #endif
  	cleanup(nfa);				/* final tidying */
+ #ifdef REG_DEBUG
+ 	if (verbose)
+ 		dumpnfa(nfa, f);
+ #endif
  	return analyze(nfa);		/* and analysis */
  }
  
*************** pull(struct nfa * nfa,
*** 1568,1574 ****
  		s = newstate(nfa);
  		if (NISERR())
  			return 0;
! 		copyins(nfa, from, s, 1);		/* duplicate inarcs */
  		cparc(nfa, con, s, to); /* move constraint arc */
  		freearc(nfa, con);
  		if (NISERR())
--- 1540,1546 ----
  		s = newstate(nfa);
  		if (NISERR())
  			return 0;
! 		copyins(nfa, from, s);	/* duplicate inarcs */
  		cparc(nfa, con, s, to); /* move constraint arc */
  		freearc(nfa, con);
  		if (NISERR())
*************** push(struct nfa * nfa,
*** 1735,1741 ****
  		s = newstate(nfa);
  		if (NISERR())
  			return 0;
! 		copyouts(nfa, to, s, 1);	/* duplicate outarcs */
  		cparc(nfa, con, from, s);		/* move constraint arc */
  		freearc(nfa, con);
  		if (NISERR())
--- 1707,1713 ----
  		s = newstate(nfa);
  		if (NISERR())
  			return 0;
! 		copyouts(nfa, to, s);	/* duplicate outarcs */
  		cparc(nfa, con, from, s);		/* move constraint arc */
  		freearc(nfa, con);
  		if (NISERR())
*************** dumpnfa(struct nfa * nfa,
*** 2952,2957 ****
--- 2924,2931 ----
  {
  #ifdef REG_DEBUG
  	struct state *s;
+ 	int			nstates = 0;
+ 	int			narcs = 0;
  
  	fprintf(f, "pre %d, post %d", nfa->pre->no, nfa->post->no);
  	if (nfa->bos[0] != COLORLESS)
*************** dumpnfa(struct nfa * nfa,
*** 2964,2970 ****
--- 2938,2949 ----
  		fprintf(f, ", eol [%ld]", (long) nfa->eos[1]);
  	fprintf(f, "\n");
  	for (s = nfa->states; s != NULL; s = s->next)
+ 	{
  		dumpstate(s, f);
+ 		nstates++;
+ 		narcs += s->nouts;
+ 	}
+ 	fprintf(f, "total of %d states, %d arcs\n", nstates, narcs);
  	if (nfa->parent == NULL)
  		dumpcolors(nfa->cm, f);
  	fflush(f);
diff --git a/src/backend/regex/regcomp.c b/src/backend/regex/regcomp.c
index 324fea5..b733bc7 100644
*** a/src/backend/regex/regcomp.c
--- b/src/backend/regex/regcomp.c
*************** static int	sortins_cmp(const void *, con
*** 136,145 ****
  static void sortouts(struct nfa *, struct state *);
  static int	sortouts_cmp(const void *, const void *);
  static void moveins(struct nfa *, struct state *, struct state *);
! static void copyins(struct nfa *, struct state *, struct state *, int);
  static void mergeins(struct nfa *, struct state *, struct arc **, int);
  static void moveouts(struct nfa *, struct state *, struct state *);
! static void copyouts(struct nfa *, struct state *, struct state *, int);
  static void cloneouts(struct nfa *, struct state *, struct state *, struct state *, int);
  static void delsub(struct nfa *, struct state *, struct state *);
  static void deltraverse(struct nfa *, struct state *, struct state *);
--- 136,145 ----
  static void sortouts(struct nfa *, struct state *);
  static int	sortouts_cmp(const void *, const void *);
  static void moveins(struct nfa *, struct state *, struct state *);
! static void copyins(struct nfa *, struct state *, struct state *);
  static void mergeins(struct nfa *, struct state *, struct arc **, int);
  static void moveouts(struct nfa *, struct state *, struct state *);
! static void copyouts(struct nfa *, struct state *, struct state *);
  static void cloneouts(struct nfa *, struct state *, struct state *, struct state *, int);
  static void delsub(struct nfa *, struct state *, struct state *);
  static void deltraverse(struct nfa *, struct state *, struct state *);
*************** static void dumpnfa(struct nfa *, FILE *
*** 181,187 ****
  #ifdef REG_DEBUG
  static void dumpstate(struct state *, FILE *);
  static void dumparcs(struct state *, FILE *);
- static int	dumprarcs(struct arc *, struct state *, FILE *, int);
  static void dumparc(struct arc *, struct state *, FILE *);
  static void dumpcnfa(struct cnfa *, FILE *);
  static void dumpcstate(int, struct cnfa *, FILE *);
--- 181,186 ----
*************** makesearch(struct vars * v,
*** 614,620 ****
  	for (s = slist; s != NULL; s = s2)
  	{
  		s2 = newstate(nfa);
! 		copyouts(nfa, s, s2, 1);
  		for (a = s->ins; a != NULL; a = b)
  		{
  			b = a->inchain;
--- 613,621 ----
  	for (s = slist; s != NULL; s = s2)
  	{
  		s2 = newstate(nfa);
! 		NOERR();
! 		copyouts(nfa, s, s2);
! 		NOERR();
  		for (a = s->ins; a != NULL; a = b)
  		{
  			b = a->inchain;
*************** dump(regex_t *re,
*** 2014,2020 ****
  	dumpcolors(&g->cmap, f);
  	if (!NULLCNFA(g->search))
  	{
! 		printf("\nsearch:\n");
  		dumpcnfa(&g->search, f);
  	}
  	for (i = 1; i < g->nlacons; i++)
--- 2015,2021 ----
  	dumpcolors(&g->cmap, f);
  	if (!NULLCNFA(g->search))
  	{
! 		fprintf(f, "\nsearch:\n");
  		dumpcnfa(&g->search, f);
  	}
  	for (i = 1; i < g->nlacons; i++)
diff --git a/src/test/regress/expected/regex.out b/src/test/regress/expected/regex.out
index 984338e..1077d7d 100644
*** a/src/test/regress/expected/regex.out
--- b/src/test/regress/expected/regex.out
*************** select 'a' ~ '((((((a+|)+|)+|)+|)+|)+|)'
*** 223,228 ****
--- 223,263 ----
   t
  (1 row)
  
+ -- These cases used to give too-many-states failures
+ select 'x' ~ 'abcd(\m)+xyz';
+  ?column? 
+ ----------
+  f
+ (1 row)
+ 
+ select 'a' ~ '^abcd*(((((^(a c(e?d)a+|)+|)+|)+|)+|a)+|)';
+  ?column? 
+ ----------
+  f
+ (1 row)
+ 
+ select 'x' ~ 'a^(^)bcd*xy(((((($a+|)+|)+|)+$|)+|)+|)^$';
+  ?column? 
+ ----------
+  f
+ (1 row)
+ 
+ select 'x' ~ 'xyz(\Y\Y)+';
+  ?column? 
+ ----------
+  f
+ (1 row)
+ 
+ select 'x' ~ 'x|(?:\M)+';
+  ?column? 
+ ----------
+  t
+ (1 row)
+ 
+ -- This generates O(N) states but O(N^2) arcs, so it causes problems
+ -- if arc count is not constrained
+ select 'x' ~ repeat('x*y*z*', 1000);
+ ERROR:  invalid regular expression: regular expression is too complex
  -- Test backref in combination with non-greedy quantifier
  -- https://core.tcl.tk/tcl/tktview/6585b21ca8fa6f3678d442b97241fdd43dba2ec0
  select 'Programmer' ~ '(\w).*?\1' as t;
diff --git a/src/test/regress/sql/regex.sql b/src/test/regress/sql/regex.sql
index 4b9c6ea..1fe052d 100644
*** a/src/test/regress/sql/regex.sql
--- b/src/test/regress/sql/regex.sql
*************** select 'dd x' ~ '(^(?!aa)(?!bb)(?!cc))+'
*** 54,59 ****
--- 54,70 ----
  select 'a' ~ '((((((a)*)*)*)*)*)*';
  select 'a' ~ '((((((a+|)+|)+|)+|)+|)+|)';
  
+ -- These cases used to give too-many-states failures
+ select 'x' ~ 'abcd(\m)+xyz';
+ select 'a' ~ '^abcd*(((((^(a c(e?d)a+|)+|)+|)+|)+|a)+|)';
+ select 'x' ~ 'a^(^)bcd*xy(((((($a+|)+|)+|)+$|)+|)+|)^$';
+ select 'x' ~ 'xyz(\Y\Y)+';
+ select 'x' ~ 'x|(?:\M)+';
+ 
+ -- This generates O(N) states but O(N^2) arcs, so it causes problems
+ -- if arc count is not constrained
+ select 'x' ~ repeat('x*y*z*', 1000);
+ 
  -- Test backref in combination with non-greedy quantifier
  -- https://core.tcl.tk/tcl/tktview/6585b21ca8fa6f3678d442b97241fdd43dba2ec0
  select 'Programmer' ~ '(\w).*?\1' as t;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to